From 07460fc320b774cbd6def67dad983932f7cad1a7 Mon Sep 17 00:00:00 2001 From: cparsons Date: Wed, 20 Jun 2018 10:41:48 -0700 Subject: Allow structField callables to specify useSkylarkSemantics, useLocation, and useEnvironment Unfortunately this doesn't work for all callers, namely NativeInfo objects, as they may have structField callables invoked from contexts that have no environment available. RELNOTES[INC]: Skylark structs (using struct()) may no longer have to_json and to_proto overridden. PiperOrigin-RevId: 201376969 --- .../build/lib/packages/StructProvider.java | 10 +++- .../processor/SkylarkCallableProcessor.java | 4 +- .../devtools/build/lib/syntax/DotExpression.java | 42 +++++++++------ .../build/lib/syntax/FuncallExpression.java | 61 ++++++++++++++++------ 4 files changed, 81 insertions(+), 36 deletions(-) (limited to 'src/main/java') diff --git a/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java b/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java index 21f847528b..4396b43a15 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StructProvider.java @@ -38,8 +38,14 @@ public final class StructProvider extends BuiltinProvider @Override public Info createStruct(SkylarkDict kwargs, Location loc) throws EvalException { - return SkylarkInfo.createSchemaless( - this, kwargs.getContents(String.class, Object.class, "kwargs"), loc); + Map kwargsMap = kwargs.getContents(String.class, Object.class, "kwargs"); + if (kwargsMap.containsKey("to_json")) { + throw new EvalException(loc, "cannot override built-in struct function 'to_json'"); + } + if (kwargsMap.containsKey("to_proto")) { + throw new EvalException(loc, "cannot override built-in struct function 'to_proto'"); + } + return SkylarkInfo.createSchemaless(this, kwargsMap, loc); } /** diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java index 71df625cba..2ccc6ccb49 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java @@ -158,14 +158,12 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { throws SkylarkCallableProcessorException { if (annotation.structField()) { if (annotation.useAst() - || annotation.useEnvironment() - || annotation.useAst() || !annotation.extraPositionals().name().isEmpty() || !annotation.extraKeywords().name().isEmpty()) { throw new SkylarkCallableProcessorException( methodElement, "@SkylarkCallable-annotated methods with structField=true may not also specify " - + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords"); + + "useAst, extraPositionals, or extraKeywords"); } } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java index ed45d6079f..b41f274c37 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/DotExpression.java @@ -118,6 +118,32 @@ public final class DotExpression extends Expression { */ public static Object eval(Object objValue, String name, Location loc, Environment env) throws EvalException, InterruptedException { + + Iterable methods = + objValue instanceof Class + ? FuncallExpression.getMethods((Class) objValue, name) + : FuncallExpression.getMethods(objValue.getClass(), name); + + if (methods != null) { + Optional method = + Streams.stream(methods) + .filter(methodDescriptor -> methodDescriptor.getAnnotation().structField()) + .findFirst(); + if (method.isPresent() && method.get().getAnnotation().structField()) { + return FuncallExpression.callMethod( + method.get(), + name, + objValue, + FuncallExpression.extraInterpreterArgs( + method.get().getAnnotation(), + /* ast = */ null, + loc, + env).toArray(), + loc, + env); + } + } + if (objValue instanceof SkylarkClassObject) { try { return ((SkylarkClassObject) objValue).getValue(name); @@ -142,22 +168,6 @@ public final class DotExpression extends Expression { } } - Iterable methods = - objValue instanceof Class - ? FuncallExpression.getMethods((Class) objValue, name) - : FuncallExpression.getMethods(objValue.getClass(), name); - - if (methods != null) { - Optional method = - Streams.stream(methods) - .filter(methodDescriptor -> methodDescriptor.getAnnotation().structField()) - .findFirst(); - if (method.isPresent() && method.get().getAnnotation().structField()) { - return FuncallExpression.callMethod( - method.get(), name, objValue, new Object[] {}, loc, env); - } - } - return null; } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java index 236bcc2576..bf43266b21 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java @@ -411,6 +411,9 @@ public final class FuncallExpression extends Expression { /** * Invokes the given structField=true method and returns the result. * + *

The given method must not require extra-interpreter parameters, such as + * {@link Environment}. This method throws {@link IllegalArgumentException} for violations.

+ * * @param methodDescriptor the descriptor of the method to invoke * @param fieldName the name of the struct field * @param obj the object on which to invoke the method @@ -420,7 +423,12 @@ public final class FuncallExpression extends Expression { public static Object invokeStructField( MethodDescriptor methodDescriptor, String fieldName, Object obj) throws EvalException, InterruptedException { - Preconditions.checkArgument(methodDescriptor.getAnnotation().structField()); + Preconditions.checkArgument(methodDescriptor.getAnnotation().structField(), + "Can only be invoked on structField callables"); + Preconditions.checkArgument(!methodDescriptor.getAnnotation().useEnvironment() + || !methodDescriptor.getAnnotation().useSkylarkSemantics() + || !methodDescriptor.getAnnotation().useLocation(), + "Cannot be invoked on structField callables with extra interpreter params"); return callMethod(methodDescriptor, fieldName, obj, new Object[0], Location.BUILTIN, null); } @@ -494,8 +502,12 @@ public final class FuncallExpression extends Expression { if (methods != null) { for (MethodDescriptor method : methods) { if (method.getAnnotation().structField()) { - // TODO(cparsons): Allow structField methods to accept interpreter-supplied arguments. - return new Pair<>(method, null); + // This indicates a built-in structField which returns a function which may have + // one or more arguments itself. For example, foo.bar('baz'), where foo.bar is a + // structField returning a function. Calling the "bar" callable of foo should + // not have 'baz' propagated, though extra interpreter arguments should be supplied. + return new Pair<>(method, + extraInterpreterArgs(method.getAnnotation(), null, getLocation(), environment)); } else { argumentListConversionResult = convertArgumentList(args, kwargs, method, environment); if (argumentListConversionResult.getArguments() != null) { @@ -566,6 +578,36 @@ public final class FuncallExpression extends Expression { return param.named() || param.legacyNamed(); } + /** + * Returns the extra interpreter arguments for the given {@link SkylarkCallable}, to be added + * at the end of the argument list for the callable. + * + *

This method accepts null {@code ast} only if {@code callable.useAst()} is false. It is + * up to the caller to validate this invariant.

+ */ + public static List extraInterpreterArgs(SkylarkCallable callable, + @Nullable FuncallExpression ast, Location loc, Environment env) { + + ImmutableList.Builder builder = ImmutableList.builder(); + + if (callable.useLocation()) { + builder.add(loc); + } + if (callable.useAst()) { + if (ast == null) { + throw new IllegalArgumentException("Callable expects to receive ast: " + callable.name()); + } + builder.add(ast); + } + if (callable.useEnvironment()) { + builder.add(env); + } + if (callable.useSkylarkSemantics()) { + builder.add(env.getSemantics()); + } + return builder.build(); + } + /** * Constructs the parameters list to actually pass to the method, filling with default values if * any. If there is a type or argument mismatch, returns a result containing an error message. @@ -702,18 +744,7 @@ public final class FuncallExpression extends Expression { if (acceptsExtraKwargs) { builder.add(SkylarkDict.copyOf(environment, extraKwargsBuilder.build())); } - if (callable.useLocation()) { - builder.add(getLocation()); - } - if (callable.useAst()) { - builder.add(this); - } - if (callable.useEnvironment()) { - builder.add(environment); - } - if (callable.useSkylarkSemantics()) { - builder.add(environment.getSemantics()); - } + builder.addAll(extraInterpreterArgs(callable, this, getLocation(), environment)); return ArgumentListConversionResult.fromArgumentList(builder.build()); } -- cgit v1.2.3