Skip to content

Commit

Permalink
bazel syntax: rename ListLiteral to ListExpression
Browse files Browse the repository at this point in the history
A literal is a value provided in the syntax tree.
By contrast, a list expression involves computation
(of its elements) and may fail.

Also:
- remove enum Kind. A boolean is fine.
- update comments and doc strings.

A follow-up will do the same for DictionaryLiteral.

PiperOrigin-RevId: 269366336
  • Loading branch information
Googler authored and copybara-github committed Sep 16, 2019
1 parent b970b24 commit b6f33cd
Show file tree
Hide file tree
Showing 12 changed files with 90 additions and 112 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
import com.google.devtools.build.lib.syntax.Identifier;
import com.google.devtools.build.lib.syntax.IfStatement;
import com.google.devtools.build.lib.syntax.IntegerLiteral;
import com.google.devtools.build.lib.syntax.ListLiteral;
import com.google.devtools.build.lib.syntax.ListExpression;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Node;
import com.google.devtools.build.lib.syntax.NodeVisitor;
Expand Down Expand Up @@ -1850,8 +1850,8 @@ public void visit(FuncallExpression node) {
continue;
}
if (arg.getIdentifier() == null || arg.getIdentifier().getName().equals("include")) {
if (arg.getValue() instanceof ListLiteral) {
ListLiteral list = (ListLiteral) arg.getValue();
if (arg.getValue() instanceof ListExpression) {
ListExpression list = (ListExpression) arg.getValue();
for (Expression elem : list.getElements()) {
if (elem instanceof StringLiteral) {
globStrings.add(((StringLiteral) elem).getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
/** Syntax node for an assignment statement. */
public final class AssignmentStatement extends Statement {

private final Expression lhs; // = IDENTIFIER | DOT | INDEX | LIST_LITERAL
private final Expression lhs; // = IDENTIFIER | DOT | INDEX | LIST_EXPR
private final Expression rhs;

/**
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/com/google/devtools/build/lib/syntax/Eval.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ static void assign(Expression expr, Object value, Environment env, Location loc)
Object object = ((IndexExpression) expr).getObject().eval(env);
Object key = ((IndexExpression) expr).getKey().eval(env);
assignItem(object, key, value, env, loc);
} else if (expr instanceof ListLiteral) {
ListLiteral list = (ListLiteral) expr;
} else if (expr instanceof ListExpression) {
ListExpression list = (ListExpression) expr;
assignList(list, value, env, loc);
} else {
// Not possible for validated ASTs.
Expand Down Expand Up @@ -287,12 +287,12 @@ private static void assignItem(
}

/**
* Recursively assigns an iterable value to a list literal.
* Recursively assigns an iterable value to a sequence of assignable expressions.
*
* @throws EvalException if the list literal has length 0, or if the value is not an iterable of
* matching length
*/
private static void assignList(ListLiteral list, Object value, Environment env, Location loc)
private static void assignList(ListExpression list, Object value, Environment env, Location loc)
throws EvalException, InterruptedException {
Collection<?> collection = EvalUtils.toCollection(value, loc, env);
int len = list.getElements().size();
Expand Down Expand Up @@ -341,7 +341,7 @@ private static void assignAugmented(
Object rhsValue = rhs.eval(env);
Object result = BinaryOperatorExpression.evaluateAugmented(op, oldValue, rhsValue, env, loc);
assignItem(object, key, result, env, loc);
} else if (expr instanceof ListLiteral) {
} else if (expr instanceof ListExpression) {
throw new EvalException(loc, "cannot perform augmented assignment on a list literal");
} else {
// Not possible for validated ASTs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public enum Kind {
IDENTIFIER,
INDEX,
INTEGER_LITERAL,
LIST_LITERAL,
LIST_EXPR,
SLICE,
STRING_LITERAL,
UNARY_OPERATOR,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ private static void collectBoundIdentifiers(
result.add((Identifier) lhs);
return;
}
if (lhs instanceof ListLiteral) {
ListLiteral variables = (ListLiteral) lhs;
if (lhs instanceof ListExpression) {
ListExpression variables = (ListExpression) lhs;
for (Expression expression : variables.getElements()) {
collectBoundIdentifiers(expression, result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,46 +19,25 @@
import java.util.ArrayList;
import java.util.List;

/**
* Syntax node for list and tuple literals.
*
* <p>(Note that during evaluation, both list and tuple values are represented by java.util.List
* objects, the only difference between them being whether or not they are mutable.)
*/
// TODO(adonovan): rename ListExpression.
public final class ListLiteral extends Expression {
/** Syntax node for list and tuple expressions. */
public final class ListExpression extends Expression {

/** Types of the ListLiteral. */
// TODO(laurentlb): This conflicts with Expression.Kind, maybe remove it?
// TODO(adonovan): split class into {List,Tuple}Expression, as a tuple may have no parens.
// (Do this after we materialize ParenExpressions.)
public enum Kind {
LIST,
TUPLE
}

private final Kind kind;

private final boolean isTuple;
private final List<Expression> elements;

ListLiteral(Kind kind, List<Expression> elements) {
this.kind = kind;
ListExpression(boolean isTuple, List<Expression> elements) {
this.isTuple = isTuple;
this.elements = elements;
}

public Kind getKind() {
return kind;
}

public List<Expression> getElements() {
return elements;
}

/**
* Returns true if this list is a tuple (a hash table, immutable list).
*/
/** Reports whether this is a tuple expression. */
public boolean isTuple() {
return kind == Kind.TUPLE;
return isTuple;
}

@Override
Expand Down Expand Up @@ -105,6 +84,6 @@ public void accept(NodeVisitor visitor) {

@Override
public Expression.Kind kind() {
return Expression.Kind.LIST_LITERAL;
return Expression.Kind.LIST_EXPR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void visit(LoadStatement node) {
}
}

public void visit(ListLiteral node) {
public void visit(ListExpression node) {
visitAll(node.getElements());
}

Expand Down
13 changes: 7 additions & 6 deletions src/main/java/com/google/devtools/build/lib/syntax/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ private Expression parseExpression(boolean insideParens) {
List<Expression> tuple = parseExprList(insideParens);
tuple.add(0, expression); // add the first expression to the front of the tuple
return setLocation(
new ListLiteral(ListLiteral.Kind.TUPLE, tuple), start, Iterables.getLast(tuple));
new ListExpression(/*isTuple=*/ true, tuple), start, Iterables.getLast(tuple));
}

private void reportError(Location location, String message) {
Expand Down Expand Up @@ -637,7 +637,7 @@ private Expression parsePrimary() {
nextToken();
// check for the empty tuple literal
if (token.kind == TokenKind.RPAREN) {
ListLiteral tuple = new ListLiteral(ListLiteral.Kind.TUPLE, ImmutableList.of());
ListExpression tuple = new ListExpression(/*isTuple=*/ true, ImmutableList.of());
setLocation(tuple, start, token.right);
nextToken();
return tuple;
Expand Down Expand Up @@ -768,7 +768,7 @@ private Expression parseForLoopVariables() {
tuple.add(parsePrimaryWithSuffix());
}
return setLocation(
new ListLiteral(ListLiteral.Kind.TUPLE, tuple), start, Iterables.getLast(tuple));
new ListExpression(/*isTuple=*/ true, tuple), start, Iterables.getLast(tuple));
}

// comprehension_suffix ::= 'FOR' loop_variables 'IN' expr comprehension_suffix
Expand Down Expand Up @@ -814,7 +814,7 @@ private Expression parseListMaker() {
int start = token.left;
expect(TokenKind.LBRACKET);
if (token.kind == TokenKind.RBRACKET) { // empty List
ListLiteral list = new ListLiteral(ListLiteral.Kind.LIST, ImmutableList.of());
ListExpression list = new ListExpression(/*isTuple=*/ false, ImmutableList.of());
setLocation(list, start, token.right);
nextToken();
return list;
Expand All @@ -825,7 +825,8 @@ private Expression parseListMaker() {
switch (token.kind) {
case RBRACKET: // singleton list
{
ListLiteral list = new ListLiteral(ListLiteral.Kind.LIST, ImmutableList.of(expression));
ListExpression list =
new ListExpression(/*isTuple=*/ false, ImmutableList.of(expression));
setLocation(list, start, token.right);
nextToken();
return list;
Expand All @@ -844,7 +845,7 @@ private Expression parseListMaker() {
token.right);
elems.add(0, expression); // TODO(adonovan): opt: don't do this
if (token.kind == TokenKind.RBRACKET) {
ListLiteral list = new ListLiteral(ListLiteral.Kind.LIST, elems);
ListExpression list = new ListExpression(/*isTuple=*/ false, elems);
setLocation(list, start, token.right);
nextToken();
return list;
Expand Down
66 changes: 32 additions & 34 deletions src/main/java/com/google/devtools/build/lib/syntax/SkylarkList.java
Original file line number Diff line number Diff line change
Expand Up @@ -207,23 +207,22 @@ public static <E> SkylarkList<E> createImmutable(Iterable<? extends E> contents)
* A Skylark list, i.e., the value represented by {@code [1, 2, 3]}. Lists are mutable datatypes.
*/
@SkylarkModule(
name = "list",
category = SkylarkModuleCategory.BUILTIN,
doc =
"A language built-in type to support lists. Example of list literal:<br>"
+ "<pre class=language-python>x = [1, 2, 3]</pre>"
+ "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+ "<pre class=language-python>e = x[1] # e == 2</pre>"
+ "Lists support the <code>+</code> operator to concatenate two lists. Example:<br>"
+ "<pre class=language-python>x = [1, 2] + [3, 4] # x == [1, 2, 3, 4]\n"
+ "x = [\"a\", \"b\"]\n"
+ "x += [\"c\"] # x == [\"a\", \"b\", \"c\"]</pre>"
+ "Similar to strings, lists support slice operations:"
+ "<pre class=language-python>['a', 'b', 'c', 'd'][1:3] # ['b', 'c']\n"
+ "['a', 'b', 'c', 'd'][::2] # ['a', 'c']\n"
+ "['a', 'b', 'c', 'd'][3:0:-1] # ['d', 'c', 'b']</pre>"
+ "Lists are mutable, as in Python."
)
name = "list",
category = SkylarkModuleCategory.BUILTIN,
doc =
"The built-in list type. Example list expressions:<br>"
+ "<pre class=language-python>x = [1, 2, 3]</pre>"
+ "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+ "<pre class=language-python>e = x[1] # e == 2</pre>"
+ "Lists support the <code>+</code> operator to concatenate two lists. Example:<br>"
+ "<pre class=language-python>x = [1, 2] + [3, 4] # x == [1, 2, 3, 4]\n"
+ "x = [\"a\", \"b\"]\n"
+ "x += [\"c\"] # x == [\"a\", \"b\", \"c\"]</pre>"
+ "Similar to strings, lists support slice operations:"
+ "<pre class=language-python>['a', 'b', 'c', 'd'][1:3] # ['b', 'c']\n"
+ "['a', 'b', 'c', 'd'][::2] # ['a', 'c']\n"
+ "['a', 'b', 'c', 'd'][3:0:-1] # ['d', 'c', 'b']</pre>"
+ "Lists are mutable, as in Python.")
public static final class MutableList<E> extends SkylarkList<E> {

private final ArrayList<E> contents;
Expand Down Expand Up @@ -604,23 +603,22 @@ public Object pop(Object i, Location loc, Environment env)
* (regardless of the {@link Environment} they are created in).
*/
@SkylarkModule(
name = "tuple",
category = SkylarkModuleCategory.BUILTIN,
doc =
"A language built-in type to support tuples. Example of tuple literal:<br>"
+ "<pre class=language-python>x = (1, 2, 3)</pre>"
+ "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+ "<pre class=language-python>e = x[1] # e == 2</pre>"
+ "Lists support the <code>+</code> operator to concatenate two tuples. Example:<br>"
+ "<pre class=language-python>x = (1, 2) + (3, 4) # x == (1, 2, 3, 4)\n"
+ "x = (\"a\", \"b\")\n"
+ "x += (\"c\",) # x == (\"a\", \"b\", \"c\")</pre>"
+ "Similar to lists, tuples support slice operations:"
+ "<pre class=language-python>('a', 'b', 'c', 'd')[1:3] # ('b', 'c')\n"
+ "('a', 'b', 'c', 'd')[::2] # ('a', 'c')\n"
+ "('a', 'b', 'c', 'd')[3:0:-1] # ('d', 'c', 'b')</pre>"
+ "Tuples are immutable, therefore <code>x[1] = \"a\"</code> is not supported."
)
name = "tuple",
category = SkylarkModuleCategory.BUILTIN,
doc =
"The built-in tuple type. Example tuple expressions:<br>"
+ "<pre class=language-python>x = (1, 2, 3)</pre>"
+ "Accessing elements is possible using indexing (starts from <code>0</code>):<br>"
+ "<pre class=language-python>e = x[1] # e == 2</pre>"
+ "Lists support the <code>+</code> operator to concatenate two tuples. Example:<br>"
+ "<pre class=language-python>x = (1, 2) + (3, 4) # x == (1, 2, 3, 4)\n"
+ "x = (\"a\", \"b\")\n"
+ "x += (\"c\",) # x == (\"a\", \"b\", \"c\")</pre>"
+ "Similar to lists, tuples support slice operations:"
+ "<pre class=language-python>('a', 'b', 'c', 'd')[1:3] # ('b', 'c')\n"
+ "('a', 'b', 'c', 'd')[::2] # ('a', 'c')\n"
+ "('a', 'b', 'c', 'd')[3:0:-1] # ('d', 'c', 'b')</pre>"
+ "Tuples are immutable, therefore <code>x[1] = \"a\"</code> is not supported.")
public static final class Tuple<E> extends SkylarkList<E> {

private final ImmutableList<E> contents;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ private void assign(Expression lhs) {
// no-op
} else if (lhs instanceof IndexExpression) {
visit(lhs);
} else if (lhs instanceof ListLiteral) {
for (Expression elem : ((ListLiteral) lhs).getElements()) {
} else if (lhs instanceof ListExpression) {
for (Expression elem : ((ListExpression) lhs).getElements()) {
assign(elem);
}
} else {
Expand Down Expand Up @@ -309,7 +309,7 @@ public void visit(AssignmentStatement node) {

@Override
public void visit(AugmentedAssignmentStatement node) {
if (node.getLHS() instanceof ListLiteral) {
if (node.getLHS() instanceof ListExpression) {
throw new ValidationException(
node.getLocation(), "cannot perform augmented assignment on a list or tuple expression");
}
Expand Down
Loading

0 comments on commit b6f33cd

Please sign in to comment.