bazel packages: remove attribute locations
Currently, along with the value of each rule attribute, Blaze records
the optional source location of the identifier of the named
argument that provides the attribute's value, such as 'srcs' in
cc_binary(..., srcs=[...]). This helps blaze emit more precise errors
for problems in a particular attribute, especially for rule
instantiation calls that span many lines.
However, this feature requires a power of introspection not found in
most programming languages: the ability to walk up the call stack to
find the caller and then inspect its syntax tree. As specified,
the Starlark calling convention provides the callee with only the
thread, the values of the positional arguments, and the names and
values of the named arguments; it does not provide source syntax.
The source syntax parameter is an obstacle to upcoming evaluator
optimizations, which will compile syntax trees to a more compact,
efficient, and easily serialized executable representation.
Therefore, this change removes the feature.
All attribute errors now report the location of the rule as a whole.
On the plus side, this change should reduce memory usage of loaded packages.
Follow-up changes will remove the FuncallExpression parameter from
the Starlark function call API once all uses have been eliminated.
PiperOrigin-RevId: 269832751
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapper.java
index 2838565..a027e80 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapper.java
@@ -17,7 +17,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.RuleClass;
@@ -182,13 +181,4 @@
&& aspectAttributes.get(attrName).getType() == type;
}
}
-
- @Override
- public Location getAttributeLocation(String attrName) {
- if (ruleAttributes.has(attrName)) {
- return ruleAttributes.getAttributeLocation(attrName);
- } else {
- return Location.BUILTIN;
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java b/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
index 8aab1c8..cddc434 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
@@ -55,7 +55,7 @@
@Override
public void attributeError(String attrName, String message) {
- reportError(getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
+ reportError(getRuleLocation(), completeAttributeMessage(attrName, message));
}
@Override
@@ -74,7 +74,7 @@
@Override
public void attributeWarning(String attrName, String message) {
- reportWarning(getAttributeLocation(attrName), completeAttributeMessage(attrName, message));
+ reportWarning(getRuleLocation(), completeAttributeMessage(attrName, message));
}
private String prefixRuleMessage(String message) {
@@ -109,6 +109,4 @@
protected abstract BuildConfiguration getConfiguration();
protected abstract Location getRuleLocation();
-
- protected abstract Location getAttributeLocation(String attrName);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index ded8ede..76e799d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -2139,12 +2139,17 @@
}
@Override
- protected String getMacroMessageAppendix(String attrName) {
+ protected String getMacroMessageAppendix(String unusedAttrName) {
+ // TODO(b/141234726): Historically this reported the location
+ // of the rule attribute in the macro call (assuming no **kwargs),
+ // but we no longer locations for individual attributes.
+ // We should record the instantiation call stack in each rule
+ // and report the position of its topmost frame here.
return rule.wasCreatedByMacro()
? String.format(
". Since this rule was created by the macro '%s', the error might have been "
- + "caused by the macro implementation in %s",
- getGeneratorFunction(), rule.getAttributeLocationWithoutMacro(attrName))
+ + "caused by the macro implementation",
+ getGeneratorFunction())
: "";
}
@@ -2166,11 +2171,6 @@
protected Location getRuleLocation() {
return rule.getLocation();
}
-
- @Override
- protected Location getAttributeLocation(String attrName) {
- return rule.getAttributeLocation(attrName);
- }
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 6e03d20..ef99584 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -48,6 +48,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
@@ -501,7 +502,7 @@
SkylarkList<?> toolchains,
String doc,
Boolean applyToGeneratingRules,
- FuncallExpression ast,
+ FuncallExpression ast, // just for getLocation(); TODO(adonovan): simplify
Environment funcallEnv,
StarlarkContext context)
throws EvalException {
@@ -644,12 +645,12 @@
}
@Override
- public Object call(Object[] args, FuncallExpression ast, Environment env)
+ public Object call(Object[] args, FuncallExpression astForLocation, Environment env)
throws EvalException, InterruptedException, ConversionException {
- SkylarkUtils.checkLoadingPhase(env, getName(), ast.getLocation());
+ Location loc = astForLocation.getLocation();
+ SkylarkUtils.checkLoadingPhase(env, getName(), loc);
if (ruleClass == null) {
- throw new EvalException(
- ast.getLocation(), "Invalid rule class hasn't been exported by a bzl file");
+ throw new EvalException(loc, "Invalid rule class hasn't been exported by a bzl file");
}
for (Attribute attribute : ruleClass.getAttributes()) {
@@ -675,20 +676,15 @@
PackageContext pkgContext = env.getThreadLocal(PackageContext.class);
if (pkgContext == null) {
throw new EvalException(
- ast.getLocation(),
+ loc,
"Cannot instantiate a rule when loading a .bzl file. "
+ "Rules may be instantiated only in a BUILD thread.");
}
RuleFactory.createAndAddRule(
- pkgContext,
- ruleClass,
- attributeValues,
- ast,
- env,
- pkgContext.getAttributeContainerFactory().apply(ruleClass));
+ pkgContext, ruleClass, attributeValues, loc, env, new AttributeContainer(ruleClass));
return Runtime.NONE;
} catch (InvalidRuleException | NameConflictException e) {
- throw new EvalException(ast.getLocation(), e.getMessage());
+ throw new EvalException(loc, e.getMessage());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
index a3919ee..076f259 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java
@@ -333,18 +333,16 @@
Revision buildToolsRevision = Revision.parseRevision(buildToolsVersion);
if (buildToolsRevision.compareTo(MIN_BUILD_TOOLS_REVISION) < 0) {
throw new EvalException(
- rule.getAttributeLocation("build_tools_version"),
+ rule.getLocation(),
String.format(
"Bazel requires Android build tools version %s or newer, %s was provided",
- MIN_BUILD_TOOLS_REVISION,
- buildToolsRevision));
+ MIN_BUILD_TOOLS_REVISION, buildToolsRevision));
}
} catch (NumberFormatException e) {
throw new EvalException(
- rule.getAttributeLocation("build_tools_version"),
+ rule.getLocation(),
String.format(
- "Bazel does not recognize Android build tools version %s",
- buildToolsVersion),
+ "Bazel does not recognize Android build tools version %s", buildToolsVersion),
e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/events/Location.java b/src/main/java/com/google/devtools/build/lib/events/Location.java
index 30224dc..f4de2ba 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Location.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Location.java
@@ -290,6 +290,11 @@
public int hashCode() {
return line * 41 + column;
}
+
+ @Override
+ public String toString() {
+ return line + ":" + column;
+ }
}
static final class BuiltinLocation extends Location {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
index 0d2632d..7d49560 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AbstractAttributeMapper.java
@@ -16,7 +16,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
import java.util.ArrayList;
import java.util.Collection;
@@ -296,9 +295,4 @@
public <T> boolean has(String attrName, Type<T> type) {
return getAttributeType(attrName) == type;
}
-
- @Override
- public Location getAttributeLocation(String attrName) {
- return attributes.getAttributeLocation(attrName);
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
index b82a76e..e15b793 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
@@ -21,7 +21,6 @@
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.CollectionUtils;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Attribute.ComputationLimiter;
import com.google.devtools.build.lib.packages.BuildType.Selector;
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
@@ -584,11 +583,6 @@
public <T> boolean has(String attrName, Type<T> type) {
return owner.has(attrName, type);
}
-
- @Override
- public Location getAttributeLocation(String attrName) {
- return owner.getAttributeLocation(attrName);
- }
};
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeContainer.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeContainer.java
index 141313f..9aeddc8 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AttributeContainer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeContainer.java
@@ -13,51 +13,37 @@
// limitations under the License.
package com.google.devtools.build.lib.packages;
-import com.google.devtools.build.lib.events.Location;
-import java.util.Arrays;
import javax.annotation.Nullable;
/**
- * Provides attribute setting and retrieval for a Rule. Encapsulating attribute access
- * here means it can be passed around independently of the Rule itself. In particular,
- * it can be consumed by independent {@link AttributeMap} instances that can apply
- * varying kinds of logic for determining the "value" of an attribute. For example,
- * a configurable attribute's "value" may be a { config --> value } dictionary
- * or a configuration-bound lookup on that dictionary, depending on the context in
- * which it's requested.
+ * Provides attribute setting and retrieval for a Rule. Encapsulating attribute access here means it
+ * can be passed around independently of the Rule itself. In particular, it can be consumed by
+ * independent {@link AttributeMap} instances that can apply varying kinds of logic for determining
+ * the "value" of an attribute. For example, a configurable attribute's "value" may be a { config
+ * --> value } dictionary or a configuration-bound lookup on that dictionary, depending on the
+ * context in which it's requested.
*
- * <p>This class provides the lowest-level access to attribute information. It is *not*
- * intended to be a robust public interface, but rather just an input to {@link AttributeMap}
- * instances. Use those instances for all domain-level attribute access.
+ * <p>This class provides the lowest-level access to attribute information. It is *not* intended to
+ * be a robust public interface, but rather just an input to {@link AttributeMap} instances. Use
+ * those instances for all domain-level attribute access.
*/
-public class AttributeContainer {
+public final class AttributeContainer {
private final RuleClass ruleClass;
// Attribute values, keyed by attribute index:
private final Object[] attributeValues;
- // Holds two lists of attribute indices.
- // The first byte gives the length of the first list.
- // The first list records which attributes were set explicitly in the BUILD file.
- // The second list ends at the end of the array.
- // The second list records which attributes have Locations, in reverse order
- // from the attributeLocations array.
- // Between the lists there may be unused zero bytes (zeros are forbidden within each list).
+ // Holds a list of attribute indices.
+ // The first byte gives the length of the list.
+ // The list records which attributes were set explicitly in the BUILD file.
+ // The list may be padded with zeros at the end.
private byte[] state;
- // Attribute locations, packed:
- private Location[] attributeLocations;
-
-
/**
* Create a container for a rule of the given rule class.
*/
public AttributeContainer(RuleClass ruleClass) {
- this(ruleClass, EMPTY_LOCATIONS);
- }
-
- AttributeContainer(RuleClass ruleClass, Location[] locations) {
int n = ruleClass.getAttributeCount();
if (n > 254) {
// We reserve the zero byte as a hole/sentinel inside state[].
@@ -67,11 +53,9 @@
this.ruleClass = ruleClass;
this.attributeValues = new Object[n];
this.state = EMPTY_STATE;
- this.attributeLocations = locations;
}
private static final byte[] EMPTY_STATE = {0};
- private static final Location[] EMPTY_LOCATIONS = {};
/**
* Returns an attribute value by name, or null on no match.
@@ -118,16 +102,6 @@
return false;
}
- private int getLocationIndex(int index) {
- int n = explicitCount();
- for (int i = state.length - 1; i > n; --i) {
- if ((0xff & state[i]) == index + 1) {
- return state.length - 1 - i;
- }
- }
- return -1;
- }
-
private void setExplicit(int index) {
if (getExplicit(index)) {
return;
@@ -138,22 +112,9 @@
state[n] = (byte) (index + 1);
}
- private int addLocationIndex(int index) {
- ensureSpace();
- for (int i = state.length - 1; ; --i) {
- if (i <= explicitCount()) {
- throw new AssertionError("ensureSpace() did not insert a zero");
- }
- if (state[i] == 0) {
- state[i] = (byte) (index + 1);
- return state.length - 1 - i;
- }
- }
- }
-
/**
* Ensures that the state[n] byte is equal to the sentinel value 0, so there is room for another
- * attribute's explicit bit to be set, or for an attribute's location to be set.
+ * attribute's explicit bit to be set.
*/
private void ensureSpace() {
int n = explicitCount() + 1;
@@ -161,25 +122,13 @@
return;
}
// Grow up to the next multiple of eight bytes, as the object will be
- // aligned to eight bytes anyway. Insert zeros between the two lists.
+ // aligned to eight bytes anyway. Pad with zeros at the end.
byte[] newState = new byte[(state.length | 7) + 1];
// Copy stored explicit attributes to the beginning of the array.
System.arraycopy(state, 0, newState, 0, n);
- // Copy stored attribute locations to the *end* of the array.
- int oldLocations = state.length - n;
- System.arraycopy(state, n, newState, newState.length - oldLocations, oldLocations);
state = newState;
}
- /**
- * Returns the location of the attribute definition for this rule, or null if not found.
- */
- public Location getAttributeLocation(String attrName) {
- Integer idx = ruleClass.getAttributeIndex(attrName);
- int locationIndex = idx != null ? getLocationIndex(idx) : -1;
- return locationIndex >= 0 ? attributeLocations[locationIndex] : null;
- }
-
Object getAttributeValue(int index) {
return attributeValues[index];
}
@@ -203,18 +152,4 @@
void setAttributeValueByName(String attrName, Object value) {
setAttributeValue(ruleClass.getAttributeByName(attrName), value, true);
}
-
- void setAttributeLocation(int attrIndex, Location location) {
- int locationIndex = getLocationIndex(attrIndex);
- if (locationIndex >= 0) {
- throw new IllegalArgumentException("already have a location for attribute "
- + ruleClass.getAttribute(attrIndex).getName() + ": " + attributeLocations[locationIndex]);
- }
- locationIndex = addLocationIndex(attrIndex);
- if (locationIndex >= attributeLocations.length) {
- // Grow by two references, as the object will be aligned to eight bytes anyway.
- attributeLocations = Arrays.copyOf(attributeLocations, attributeLocations.length + 2);
- }
- attributeLocations[locationIndex] = location;
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java
index 27e2026..fcefbd3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeMap.java
@@ -16,7 +16,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Location;
import java.util.Collection;
import javax.annotation.Nullable;
@@ -109,9 +108,6 @@
*/
boolean isAttributeValueExplicitlySpecified(String attributeName);
- /** Returns the {@link Location} at which the attribute was defined. */
- Location getAttributeLocation(String attrName);
-
/**
* Returns a {@link Collection} with a {@link DepEdge} for every attribute that contains labels in
* its value (either by *being* a label or being a collection that includes labels).
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java
index b884d6a..5987fa0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ConfiguredAttributeMapper.java
@@ -171,7 +171,7 @@
if (matchingConditions.size() > 1) {
throw new EvalException(
- rule.getAttributeLocation(attributeName),
+ rule.getLocation(),
"Illegal ambiguous match on configurable attribute \""
+ attributeName
+ "\" in "
@@ -194,7 +194,7 @@
noMatchMessage += " (would a default condition help?).\nConditions checked:\n "
+ Joiner.on("\n ").join(conditionLabels);
}
- throw new EvalException(rule.getAttributeLocation(attributeName), noMatchMessage);
+ throw new EvalException(rule.getLocation(), noMatchMessage);
}
matchingResult = selector.hasDefault()
? new ConfigKeyAndValue<>(Selector.DEFAULT_CONDITION_LABEL, selector.getDefault())
diff --git a/src/main/java/com/google/devtools/build/lib/packages/DelegatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/DelegatingAttributeMapper.java
index 24e64aeb..b5f8022 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/DelegatingAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/DelegatingAttributeMapper.java
@@ -16,7 +16,6 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Location;
import java.util.Collection;
import javax.annotation.Nullable;
@@ -117,9 +116,4 @@
public <T> boolean has(String attrName, Type<T> type) {
return delegate.has(attrName, type);
}
-
- @Override
- public Location getAttributeLocation(String attrName) {
- return delegate.getAttributeLocation(attrName);
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
index fc22881..86c92ac 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ImplicitOutputsFunction.java
@@ -542,7 +542,7 @@
for (String placeholder : parsedTemplate.attributeNames()) {
if (rule.isConfigurable(placeholder)) {
throw new EvalException(
- rule.getAttributeLocation(placeholder),
+ /*location=*/ null,
String.format(
"Attribute %s is configurable and cannot be used in outputs", placeholder));
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index 0eae995..1d3357c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1474,10 +1474,7 @@
// All labels mentioned in a rule that refer to an unknown target in the
// current package are assumed to be InputFiles, so let's create them:
for (AttributeMap.DepEdge depEdge : AggregatingAttributeMapper.of(rule).visitLabels()) {
- InputFile inputFile =
- createInputFileMaybe(
- depEdge.getLabel(),
- rule.getAttributeLocation(depEdge.getAttribute().getName()));
+ InputFile inputFile = createInputFileMaybe(depEdge.getLabel(), rule.getLocation());
if (inputFile != null && !newInputFiles.containsKey(depEdge.getLabel())) {
newInputFiles.put(depEdge.getLabel(), inputFile);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 39f135b..8462785 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -15,7 +15,6 @@
package com.google.devtools.build.lib.packages;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
@@ -363,8 +362,6 @@
public abstract static class BuilderForTesting {
protected final String version = "test";
protected Iterable<EnvironmentExtension> environmentExtensions = ImmutableList.of();
- protected Function<RuleClass, AttributeContainer> attributeContainerFactory =
- AttributeContainer::new;
protected boolean doChecksForTesting = true;
public BuilderForTesting setEnvironmentExtensions(
@@ -397,11 +394,10 @@
*/
public PackageFactory(
RuleClassProvider ruleClassProvider,
- Function<RuleClass, AttributeContainer> attributeContainerFactory,
Iterable<EnvironmentExtension> environmentExtensions,
String version,
Package.Builder.Helper packageBuilderHelper) {
- this.ruleFactory = new RuleFactory(ruleClassProvider, attributeContainerFactory);
+ this.ruleFactory = new RuleFactory(ruleClassProvider);
this.ruleFunctions = buildRuleFunctions(ruleFactory);
this.ruleClassProvider = ruleClassProvider;
setGlobbingThreads(100);
@@ -1283,13 +1279,12 @@
*/
private static class BuiltinRuleFunction extends BuiltinFunction implements RuleFunction {
private final String ruleClassName;
- private final RuleFactory ruleFactory;
private final RuleClass ruleClass;
BuiltinRuleFunction(String ruleClassName, RuleFactory ruleFactory) {
- super(ruleClassName, FunctionSignature.KWARGS, BuiltinFunction.USE_AST_ENV, /*isRule=*/ true);
+ super(ruleClassName, FunctionSignature.KWARGS, BuiltinFunction.USE_LOC_ENV, /*isRule=*/ true);
this.ruleClassName = ruleClassName;
- this.ruleFactory = Preconditions.checkNotNull(ruleFactory, "ruleFactory was null");
+ Preconditions.checkNotNull(ruleFactory, "ruleFactory was null");
this.ruleClass = Preconditions.checkNotNull(
ruleFactory.getRuleClass(ruleClassName),
"No such rule class: %s",
@@ -1297,30 +1292,25 @@
}
@SuppressWarnings({"unchecked", "unused"})
- public Runtime.NoneType invoke(
- Map<String, Object> kwargs, FuncallExpression ast, Environment env)
+ public Runtime.NoneType invoke(Map<String, Object> kwargs, Location loc, Environment env)
throws EvalException, InterruptedException {
- SkylarkUtils.checkLoadingOrWorkspacePhase(env, ruleClassName, ast.getLocation());
+ SkylarkUtils.checkLoadingOrWorkspacePhase(env, ruleClassName, loc);
try {
- addRule(getContext(env, ast.getLocation()), kwargs, ast, env);
+ addRule(getContext(env, loc), kwargs, loc, env);
} catch (RuleFactory.InvalidRuleException | Package.NameConflictException e) {
- throw new EvalException(ast.getLocation(), e.getMessage());
+ throw new EvalException(loc, e.getMessage());
}
return Runtime.NONE;
}
private void addRule(
- PackageContext context,
- Map<String, Object> kwargs,
- FuncallExpression ast,
- Environment env)
- throws RuleFactory.InvalidRuleException, Package.NameConflictException,
- InterruptedException {
+ PackageContext context, Map<String, Object> kwargs, Location loc, Environment env)
+ throws RuleFactory.InvalidRuleException, Package.NameConflictException,
+ InterruptedException {
BuildLangTypedAttributeValuesMap attributeValues =
new BuildLangTypedAttributeValuesMap(kwargs);
- AttributeContainer attributeContainer = ruleFactory.getAttributeContainer(ruleClass);
RuleFactory.createAndAddRule(
- context, ruleClass, attributeValues, ast, env, attributeContainer);
+ context, ruleClass, attributeValues, loc, env, new AttributeContainer(ruleClass));
}
@Override
@@ -1579,18 +1569,13 @@
final Package.Builder pkgBuilder;
final Globber globber;
final ExtendedEventHandler eventHandler;
- private final Function<RuleClass, AttributeContainer> attributeContainerFactory;
@VisibleForTesting
public PackageContext(
- Package.Builder pkgBuilder,
- Globber globber,
- ExtendedEventHandler eventHandler,
- Function<RuleClass, AttributeContainer> attributeContainerFactory) {
+ Package.Builder pkgBuilder, Globber globber, ExtendedEventHandler eventHandler) {
this.pkgBuilder = pkgBuilder;
this.eventHandler = eventHandler;
this.globber = globber;
- this.attributeContainerFactory = attributeContainerFactory;
}
/**
@@ -1613,10 +1598,6 @@
public Package.Builder getBuilder() {
return pkgBuilder;
}
-
- public Function<RuleClass, AttributeContainer> getAttributeContainerFactory() {
- return attributeContainerFactory;
- }
}
private final ClassObject nativeModule;
@@ -1762,9 +1743,7 @@
}
// Stuff that closes over the package context:
- PackageContext context =
- new PackageContext(
- pkgBuilder, globber, eventHandler, ruleFactory.getAttributeContainerFactory());
+ PackageContext context = new PackageContext(pkgBuilder, globber, eventHandler);
buildPkgEnv(pkgEnv, context);
if (!validatePackageIdentifier(packageId, file.getLocation(), eventHandler)) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 97090c7..33f47ff 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -131,10 +131,6 @@
attributes.setAttributeValueByName(attrName, value);
}
- void setAttributeLocation(int attrIndex, Location location) {
- attributes.setAttributeLocation(attrIndex, location);
- }
-
void setContainsErrors() {
this.containsErrors = true;
}
@@ -344,50 +340,6 @@
}
/**
- * Returns the location of the attribute definition for this rule, if known;
- * or the location of the whole rule otherwise. "attrName" need not be a
- * valid attribute name for this rule.
- *
- * <p>This method ignores whether the present rule was created by a macro or not.
- */
- public Location getAttributeLocationWithoutMacro(String attrName) {
- return getAttributeLocation(attrName, /* useBuildLocation= */ false);
- }
-
- /**
- * Returns the location of the attribute definition for this rule, if known;
- * or the location of the whole rule otherwise. "attrName" need not be a
- * valid attribute name for this rule.
- *
- * <p>If this rule was created by a macro, this method returns the
- * location of the macro invocation in the BUILD file instead.
- */
- public Location getAttributeLocation(String attrName) {
- return getAttributeLocation(attrName, /* useBuildLocation= */ true);
- }
-
- private Location getAttributeLocation(String attrName, boolean useBuildLocation) {
- /*
- * If the rule was created by a macro, we have to deal with two locations: one in the BUILD
- * file where the macro is invoked and one in the bzl file where the rule is created.
- * For error reporting, we are usually more interested in the former one.
- * Different methods in this class refer to different locations, though:
- * - getLocation() points to the location of the macro invocation in the BUILD file (thanks to
- * RuleFactory).
- * - attributes.getAttributeLocation() points to the location in the bzl file.
- */
- if (wasCreatedByMacro() && useBuildLocation) {
- return getLocation();
- }
-
- Location attrLocation = null;
- if (!attrName.equals("name")) {
- attrLocation = attributes.getAttributeLocation(attrName);
- }
- return attrLocation != null ? attrLocation : getLocation();
- }
-
- /**
* Returns whether this rule was created by a macro.
*/
public boolean wasCreatedByMacro() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 8ca63fc..f4e1d18 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -55,10 +55,8 @@
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
-import com.google.devtools.build.lib.syntax.Argument;
import com.google.devtools.build.lib.syntax.BaseFunction;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.util.FileTypeSet;
@@ -1876,7 +1874,6 @@
Label ruleLabel,
AttributeValues<T> attributeValues,
EventHandler eventHandler,
- @Nullable FuncallExpression ast,
Location location,
AttributeContainer attributeContainer,
boolean checkThirdPartyRulesHaveLicenses)
@@ -1885,9 +1882,6 @@
populateRuleAttributeValues(rule, pkgBuilder, attributeValues, eventHandler);
checkAspectAllowedValues(rule, eventHandler);
rule.populateOutputFiles(eventHandler, pkgBuilder);
- if (ast != null) {
- populateAttributeLocations(rule, ast);
- }
checkForDuplicateLabels(rule, eventHandler);
boolean actuallyCheckLicense;
@@ -2021,19 +2015,6 @@
return definedAttrIndices;
}
- /** Populates attribute locations for attributes defined in {@code ast}. */
- private void populateAttributeLocations(Rule rule, FuncallExpression ast) {
- for (Argument.Passed arg : ast.getArguments()) {
- if (arg.isKeyword()) {
- String name = arg.getName();
- Integer attrIndex = getAttributeIndex(name);
- if (attrIndex != null) {
- rule.setAttributeLocation(attrIndex, arg.getValue().getLocation());
- }
- }
- }
- }
-
/**
* Populates the attributes table of the new {@link Rule} with default values provided by this
* {@link RuleClass} and the {@code pkgBuilder}. This will only provide values for attributes that
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
index 29aace6..0530c17 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.packages;
-import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
@@ -49,15 +48,9 @@
* Maps rule class name to the metaclass instance for that rule.
*/
private final ImmutableMap<String, RuleClass> ruleClassMap;
- private final Function<RuleClass, AttributeContainer> attributeContainerFactory;
- /**
- * Constructs a RuleFactory instance.
- */
- public RuleFactory(
- RuleClassProvider provider,
- Function<RuleClass, AttributeContainer> attributeContainerFactory) {
- this.attributeContainerFactory = attributeContainerFactory;
+ /** Constructs a RuleFactory instance. */
+ public RuleFactory(RuleClassProvider provider) {
this.ruleClassMap = ImmutableMap.copyOf(provider.getRuleClassMap());
}
@@ -75,14 +68,6 @@
return ruleClassMap.get(ruleClassName);
}
- AttributeContainer getAttributeContainer(RuleClass ruleClass) {
- return attributeContainerFactory.apply(ruleClass);
- }
-
- Function<RuleClass, AttributeContainer> getAttributeContainerFactory() {
- return attributeContainerFactory;
- }
-
/**
* Creates and returns a rule instance.
*
@@ -94,7 +79,6 @@
RuleClass ruleClass,
BuildLangTypedAttributeValuesMap attributeValues,
EventHandler eventHandler,
- @Nullable FuncallExpression ast,
Location location,
@Nullable Environment env,
AttributeContainer attributeContainer)
@@ -141,7 +125,6 @@
label,
generator.attributes,
eventHandler,
- ast,
generator.location,
attributeContainer,
checkThirdPartyLicenses);
@@ -162,7 +145,6 @@
* be a map entry for each non-optional attribute of this class of rule.
* @param eventHandler a eventHandler on which errors and warnings are reported during
* rule creation
- * @param ast the abstract syntax tree of the rule expression (optional)
* @param location the location at which this rule was declared
* @param env the lexical environment of the function call which declared this rule (optional)
* @param attributeContainer the {@link AttributeContainer} the rule will contain
@@ -177,7 +159,6 @@
RuleClass ruleClass,
BuildLangTypedAttributeValuesMap attributeValues,
EventHandler eventHandler,
- @Nullable FuncallExpression ast,
Location location,
@Nullable Environment env,
AttributeContainer attributeContainer)
@@ -188,7 +169,6 @@
ruleClass,
attributeValues,
eventHandler,
- ast,
location,
env,
attributeContainer);
@@ -202,12 +182,11 @@
* @param context the package-building context in which this rule was declared
* @param ruleClass the {@link RuleClass} of the rule
* @param attributeValues a {@link BuildLangTypedAttributeValuesMap} mapping attribute names to
- * attribute values of build-language type. Each attribute must be defined for this class
- * of rule, and have a build-language-typed value which can be converted to the appropriate
- * native type of the attribute (i.e. via {@link BuildType#selectableConvert}). There must
- * be a map entry for each non-optional attribute of this class of rule.
- * @param ast the abstract syntax tree of the rule expression (mandatory because this looks up a
- * {@link Location} from the {@code ast})
+ * attribute values of build-language type. Each attribute must be defined for this class of
+ * rule, and have a build-language-typed value which can be converted to the appropriate
+ * native type of the attribute (i.e. via {@link BuildType#selectableConvert}). There must be
+ * a map entry for each non-optional attribute of this class of rule.
+ * @param loc the location of the rule expression
* @param env the lexical environment of the function call which declared this rule (optional)
* @param attributeContainer the {@link AttributeContainer} the rule will contain
* @throws InvalidRuleException if the rule could not be constructed for any reason (e.g. no
@@ -220,7 +199,7 @@
PackageContext context,
RuleClass ruleClass,
BuildLangTypedAttributeValuesMap attributeValues,
- FuncallExpression ast,
+ Location loc,
@Nullable Environment env,
AttributeContainer attributeContainer)
throws InvalidRuleException, NameConflictException, InterruptedException {
@@ -229,8 +208,7 @@
ruleClass,
attributeValues,
context.eventHandler,
- ast,
- ast.getLocation(),
+ loc,
env,
attributeContainer);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index cedf212..ea3ae1d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -117,7 +117,7 @@
this.workspaceDir = workspaceDir;
this.defaultSystemJavabaseDir = defaultSystemJavabaseDir;
this.environmentExtensions = environmentExtensions;
- this.ruleFactory = new RuleFactory(ruleClassProvider, AttributeContainer::new);
+ this.ruleFactory = new RuleFactory(ruleClassProvider);
this.workspaceGlobals = new WorkspaceGlobals(allowOverride, ruleFactory);
this.workspaceFunctions =
WorkspaceFactory.createWorkspaceFunctions(
@@ -370,7 +370,7 @@
}
workspaceEnv.setThreadLocal(
PackageFactory.PackageContext.class,
- new PackageFactory.PackageContext(builder, null, localReporter, AttributeContainer::new));
+ new PackageFactory.PackageContext(builder, null, localReporter));
} catch (EvalException e) {
throw new AssertionError(e);
}
@@ -403,7 +403,7 @@
}
static ClassObject newNativeModule(RuleClassProvider ruleClassProvider, String version) {
- RuleFactory ruleFactory = new RuleFactory(ruleClassProvider, AttributeContainer::new);
+ RuleFactory ruleFactory = new RuleFactory(ruleClassProvider);
WorkspaceGlobals workspaceGlobals = new WorkspaceGlobals(false, ruleFactory);
return WorkspaceFactory.newNativeModule(
WorkspaceFactory.createWorkspaceFunctions(false, ruleFactory, workspaceGlobals), version);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
index 5847035..54813b5 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactoryHelper.java
@@ -51,7 +51,7 @@
RuleClass ruleClass,
RuleClass bindRuleClass,
Map<String, Object> kwargs,
- FuncallExpression ast,
+ FuncallExpression ast, // just for getLocation(); TODO(adonovan): simplify
String definitionInfo)
throws RuleFactory.InvalidRuleException, Package.NameConflictException, LabelSyntaxException,
InterruptedException {
@@ -64,7 +64,6 @@
ruleClass,
attributeValues,
eventHandler,
- ast,
ast.getLocation(),
/*env=*/ null,
new AttributeContainer(ruleClass));
@@ -179,7 +178,6 @@
bindRuleClass,
attributeValues,
handler,
- /*ast=*/ null,
location,
/*env=*/ null,
attributeContainer);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
index b648f1f..a9587a1 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
@@ -255,7 +255,7 @@
nameLabel,
actual == NONE ? null : Label.parseAbsolute((String) actual, ImmutableMap.of()),
ast.getLocation(),
- ruleFactory.getAttributeContainer(ruleClass));
+ new AttributeContainer(ruleClass));
} catch (RuleFactory.InvalidRuleException
| Package.NameConflictException
| LabelSyntaxException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java
index 3780d73..6acccc2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/WorkspaceAttributeMapper.java
@@ -51,8 +51,7 @@
try {
return type.cast(value);
} catch (ClassCastException e) {
- throw new EvalException(
- rule.getAttributeContainer().getAttributeLocation(attributeName), e.getMessage());
+ throw new EvalException(rule.getLocation(), e.getMessage());
}
}
@@ -72,8 +71,7 @@
} else {
message = "select() cannot be used in macros called from WORKSPACE files";
}
- throw new EvalException(
- rule.getAttributeContainer().getAttributeLocation(attributeName), message);
+ throw new EvalException(rule.getLocation(), message);
}
return value;
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 5e4dec6..3254a90 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -1533,7 +1533,6 @@
PackageFactory packageFactory =
new PackageFactory(
ruleClassProvider,
- serverBuilder.getAttributeContainerFactory(),
serverBuilder.getEnvironmentExtensions(),
BlazeVersionInfo.instance().getVersion(),
packageBuilderHelper);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java
index 0e7cc11..6ded39d 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/ServerBuilder.java
@@ -14,13 +14,10 @@
package com.google.devtools.build.lib.runtime;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.PackageFactory;
-import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.query2.QueryEnvironmentFactory;
import com.google.devtools.build.lib.query2.common.AbstractBlazeQueryEnvironment;
import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
@@ -35,7 +32,6 @@
public final class ServerBuilder {
private QueryEnvironmentFactory queryEnvironmentFactory;
private final InvocationPolicy.Builder invocationPolicyBuilder = InvocationPolicy.newBuilder();
- private Function<RuleClass, AttributeContainer> attributeContainerFactory;
private final ImmutableList.Builder<BlazeCommand> commands = ImmutableList.builder();
private final ImmutableMap.Builder<String, InfoItem> infoItems = ImmutableMap.builder();
private final ImmutableList.Builder<QueryFunction> queryFunctions = ImmutableList.builder();
@@ -61,10 +57,6 @@
return invocationPolicyBuilder.build();
}
- Function<RuleClass, AttributeContainer> getAttributeContainerFactory() {
- return attributeContainerFactory == null ? AttributeContainer::new : attributeContainerFactory;
- }
-
ImmutableMap<String, InfoItem> getInfoItems() {
return infoItems.build();
}
@@ -117,21 +109,6 @@
}
/**
- * Sets a factory for creating {@link AttributeContainer} instances. Only one factory per server
- * is allowed. If none is set, the server uses the default implementation.
- */
- public ServerBuilder setAttributeContainerFactory(
- Function<RuleClass, AttributeContainer> attributeContainerFactory) {
- Preconditions.checkState(
- this.attributeContainerFactory == null,
- "At most one attribute container factory supported. But found two: %s and %s",
- this.attributeContainerFactory,
- attributeContainerFactory);
- this.attributeContainerFactory = Preconditions.checkNotNull(attributeContainerFactory);
- return this;
- }
-
- /**
* Adds the given command to the server. This overload only exists to avoid array object creation
* in the common case.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index 69072d4..5c3e68f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -32,7 +32,6 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.packages.AstParseResult;
-import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.BuildFileName;
import com.google.devtools.build.lib.packages.CachingPackageLocator;
@@ -266,7 +265,6 @@
pkgFactory =
new PackageFactory(
ruleClassProvider,
- AttributeContainer::new,
getEnvironmentExtensions(),
"PackageLoader",
Package.Builder.DefaultHelper.INSTANCE);
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 8ce9d1c..26c618a 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1242,9 +1242,10 @@
" deps = [':genlib'])");
update("//foo");
- assertContainsEvent("WARNING /workspace/foo/BUILD:8:12: in deps attribute of custom_rule rule "
- + "//foo:foo: genrule rule '//foo:genlib' is unexpected here (expected java_library or "
- + "java_binary); continuing anyway");
+ assertContainsEvent(
+ "WARNING /workspace/foo/BUILD:6:1: in deps attribute of custom_rule rule "
+ + "//foo:foo: genrule rule '//foo:genlib' is unexpected here (expected java_library or "
+ + "java_binary); continuing anyway");
}
@Test
@@ -1304,8 +1305,9 @@
" deps = [':genlib'])");
update("//foo");
- assertContainsEvent("WARNING /workspace/foo/BUILD:8:12: in deps attribute of custom_rule rule "
- + "//foo:foo: genrule rule '//foo:genlib' is unexpected here; continuing anyway");
+ assertContainsEvent(
+ "WARNING /workspace/foo/BUILD:6:1: in deps attribute of custom_rule rule "
+ + "//foo:foo: genrule rule '//foo:genlib' is unexpected here; continuing anyway");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/StampTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/StampTest.java
index ef8696c..3866ba2 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/config/StampTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/config/StampTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleFactory;
@@ -36,8 +35,7 @@
*/
@Test
public void testNoStampingForTests() throws Exception {
- RuleFactory ruleFactory =
- new RuleFactory(analysisMock.createRuleClassProvider(), AttributeContainer::new);
+ RuleFactory ruleFactory = new RuleFactory(analysisMock.createRuleClassProvider());
for (String name : ruleFactory.getRuleClassNames()) {
RuleClass ruleClass = ruleFactory.getRuleClass(name);
if (TargetUtils.isTestRuleName(name) && ruleClass.hasAttr("stamp", BuildType.TRISTATE)) {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/AttributeContainerTest.java b/src/test/java/com/google/devtools/build/lib/packages/AttributeContainerTest.java
index aeb5337..a632f10 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/AttributeContainerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/AttributeContainerTest.java
@@ -15,8 +15,6 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.events.Location.LineAndColumn;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import java.util.Arrays;
import java.util.Collections;
@@ -74,75 +72,38 @@
assertThat(container.isAttributeValueExplicitlySpecified(attribute2)).isFalse();
}
- private static Location newLocation() {
- return Location.fromPathAndStartColumn(null, 0, 0, new LineAndColumn(0, 0));
- }
-
- @Test
- public void testAttributeLocation() throws Exception {
- Location location1 = newLocation();
- Location location2 = newLocation();
- container.setAttributeLocation(ruleClass.getAttributeIndex(attribute1.getName()), location1);
- container.setAttributeLocation(ruleClass.getAttributeIndex(attribute2.getName()), location2);
- assertThat(container.getAttributeLocation(attribute1.getName())).isEqualTo(location1);
- assertThat(container.getAttributeLocation(attribute2.getName())).isEqualTo(location2);
- assertThat(container.getAttributeLocation("nomatch")).isNull();
- }
-
-
@Test
public void testPackedState() throws Exception {
Random rng = new Random();
// The state packing machinery has special behavior at multiples of 8,
- // so set enough explicit values and locations to exercise that.
+ // so set enough explicit values to exercise that.
final int numAttributes = 17;
Attribute[] attributes = new Attribute[numAttributes];
for (int attributeIndex = 0; attributeIndex < numAttributes; ++attributeIndex) {
attributes[attributeIndex] = ruleClass.getAttribute(attributeIndex);
}
- Location[] locations = new Location[numAttributes];
- for (int attributeIndex = 0; attributeIndex < numAttributes; ++attributeIndex) {
- locations[attributeIndex] = newLocation();
- }
- // Test relies on checking reference inequality
- assertThat(locations[0] != locations[1]).isTrue();
-
Object someValue = new Object();
for (int explicitCount = 0; explicitCount <= numAttributes; ++explicitCount) {
- for (int locationCount = 0; locationCount <= numAttributes; ++locationCount) {
AttributeContainer container = new AttributeContainer(ruleClass);
// Shuffle the attributes each time through, to exercise
// different stored indices and orderings.
Collections.shuffle(Arrays.asList(attributes));
// Also randomly interleave calls to the two setters.
int valuePassKey = rng.nextInt(1 << numAttributes);
- int locationPassKey = rng.nextInt(1 << numAttributes);
for (int pass = 0; pass <= 1; ++pass) {
for (int i = 0; i < explicitCount; ++i) {
if (pass == ((valuePassKey >> i) & 1)) {
container.setAttributeValue(attributes[i], someValue, true);
}
}
- for (int i = 0; i < locationCount; ++i) {
- if (pass == ((locationPassKey >> i) & 1)) {
- container.setAttributeLocation(
- ruleClass.getAttributeIndex(attributes[i].getName()),
- locations[i]);
- }
- }
}
for (int i = 0; i < numAttributes; ++i) {
boolean expected = i < explicitCount;
assertThat(container.isAttributeValueExplicitlySpecified(attributes[i]))
.isEqualTo(expected);
-
- Location expectedLocation = i < locationCount ? locations[i] : null;
- assertThat(container.getAttributeLocation(attributes[i].getName()))
- .isSameInstanceAs(expectedLocation);
}
- }
}
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index b482785..a17bcdc 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -788,7 +788,6 @@
ruleLabel,
new BuildLangTypedAttributeValuesMap(attributeValues),
reporter,
- /*ast=*/ null,
location,
new AttributeContainer(ruleClass),
/*checkThirdPartyRulesHaveLicenses=*/ true);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
index 2da595a..df4b0d0 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java
@@ -42,7 +42,7 @@
public class RuleFactoryTest extends PackageLoadingTestCase {
private ConfiguredRuleClassProvider provider = TestRuleClassProvider.getRuleClassProvider();
- private RuleFactory ruleFactory = new RuleFactory(provider, AttributeContainer::new);
+ private final RuleFactory ruleFactory = new RuleFactory(provider);
public static final Location LOCATION_42 = Location.fromFileAndOffsets(null, 42, 42);
@@ -65,7 +65,6 @@
ruleClass,
new BuildLangTypedAttributeValuesMap(attributeValues),
new Reporter(new EventBus()),
- /*ast=*/ null,
LOCATION_42,
/*env=*/ null,
new AttributeContainer(ruleClass));
@@ -118,7 +117,6 @@
ruleClass,
new BuildLangTypedAttributeValuesMap(attributeValues),
new Reporter(new EventBus()),
- /*ast=*/ null,
Location.fromFileAndOffsets(myPkgPath.asFragment(), 42, 42),
/*env=*/ null,
new AttributeContainer(ruleClass));
@@ -147,7 +145,6 @@
ruleClass,
new BuildLangTypedAttributeValuesMap(attributeValues),
new Reporter(new EventBus()),
- /*ast=*/ null,
LOCATION_42,
/*env=*/ null,
new AttributeContainer(ruleClass)));
@@ -176,7 +173,6 @@
ruleClass,
new BuildLangTypedAttributeValuesMap(attributeValues),
new Reporter(new EventBus()),
- /*ast=*/ null,
Location.fromFileAndOffsets(myPkgPath.asFragment(), 42, 42),
/*env=*/ null,
new AttributeContainer(ruleClass)));
@@ -217,7 +213,6 @@
ruleClass,
new BuildLangTypedAttributeValuesMap(attributeValues),
new Reporter(new EventBus()),
- /*ast=*/ null,
Location.fromFileAndOffsets(myPkgPath.asFragment(), 42, 42),
/*env=*/ null,
new AttributeContainer(ruleClass)));
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java
index 936baa6..324f3e1 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.devtools.build.lib.events.Location.LineAndColumn;
import com.google.devtools.build.lib.events.util.EventCollectionApparatus;
import com.google.devtools.build.lib.packages.util.PackageFactoryApparatus;
import com.google.devtools.build.lib.testutil.Scratch;
@@ -44,37 +43,6 @@
}
@Test
- public void testAttributeLocation() throws Exception {
- Path buildFile = scratch.file("x/BUILD",
- "cc_binary(name = 'x',",
- " srcs = ['a', 'b', 'c'],",
- " defines = ['-Da', '-Db'])");
- Package pkg = packages.createPackage("x", RootedPath.toRootedPath(root, buildFile));
- Rule rule = pkg.getRule("x");
-
- assertThat(rule.getLocation().getStartLineAndColumn()).isEqualTo(new LineAndColumn(1, 1));
-
- // Special "name" attribute always has same location as rule:
- assertThat(rule.getAttributeLocation("name").getStartLineAndColumn())
- .isEqualTo(new LineAndColumn(1, 1));
-
- // User-provided attributes have precise locations:
- assertThat(rule.getAttributeLocation("srcs").getStartLineAndColumn())
- .isEqualTo(new LineAndColumn(2, 18));
- assertThat(rule.getAttributeLocation("defines").getStartLineAndColumn())
- .isEqualTo(new LineAndColumn(3, 21));
-
- // Default attributes have same location as rule:
- assertThat(rule.getAttributeLocation("malloc").getStartLineAndColumn())
- .isEqualTo(new LineAndColumn(1, 1));
-
- // Attempts to locate non-existent attributes don't fail;
- // the rule location is returned:
- assertThat(rule.getAttributeLocation("no-such-attr").getStartLineAndColumn())
- .isEqualTo(new LineAndColumn(1, 1));
- }
-
- @Test
public void testOutputNameError() throws Exception {
events.setFailFast(false);
Path buildFile = scratch.file("namecollide/BUILD",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
index cc33b98..bfb8ebd 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java
@@ -21,7 +21,6 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
-import com.google.devtools.build.lib.packages.AttributeContainer;
import com.google.devtools.build.lib.packages.CachingPackageLocator;
import com.google.devtools.build.lib.packages.ConstantRuleVisibility;
import com.google.devtools.build.lib.packages.GlobCache;
@@ -60,7 +59,6 @@
factory =
new PackageFactory(
ruleClassProvider,
- AttributeContainer::new,
ImmutableList.copyOf(environmentExtensions),
"test",
Package.Builder.DefaultHelper.INSTANCE);
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index d183e31..ff632c9 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -176,7 +176,7 @@
assertContainsEvent(
". Since this "
+ "rule was created by the macro 'macro_native_rule', the error might have been caused "
- + "by the macro implementation in /workspace/test/macros.bzl:10:41");
+ + "by the macro implementation");
}
@Test
@@ -188,7 +188,7 @@
+ "//test:m_skylark: '//test:jlib' does not have mandatory providers:"
+ " 'some_provider'. "
+ "Since this rule was created by the macro 'macro_skylark_rule', the error might "
- + "have been caused by the macro implementation in /workspace/test/macros.bzl:12:36");
+ + "have been caused by the macro implementation");
}
@Test
@@ -206,7 +206,7 @@
setUpAttributeErrorTest();
assertThrows(Exception.class, () -> createRuleContext("//test:skyrule"));
assertContainsEvent(
- "ERROR /workspace/test/BUILD:11:10: in deps attribute of "
+ "ERROR /workspace/test/BUILD:10:1: in deps attribute of "
+ "skylark_rule rule //test:skyrule: '//test:jlib' does not have mandatory providers: "
+ "'some_provider'");
}
@@ -247,7 +247,7 @@
assertThrows(Exception.class, () -> createRuleContext("//test:skyrule2"));
assertContainsEvent(
- "ERROR /workspace/test/BUILD:9:10: in deps attribute of "
+ "ERROR /workspace/test/BUILD:8:1: in deps attribute of "
+ "skylark_rule rule //test:skyrule2: '//test:my_other_lib' does not have "
+ "mandatory providers: 'a' or 'c'");
}
@@ -279,7 +279,7 @@
assertThrows(Exception.class, () -> createRuleContext("//test:skyrule2"));
assertContainsEvent(
- "ERROR /workspace/test/BUILD:9:10: in deps attribute of "
+ "ERROR /workspace/test/BUILD:8:1: in deps attribute of "
+ "testing_rule_for_mandatory_providers rule //test:skyrule2: '//test:my_other_lib' "
+ "does not have mandatory providers: 'a' or 'c'");
}
@@ -294,7 +294,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:cclib");
assertContainsEvent(
- "ERROR /workspace/test/BUILD:2:10: Label '//test:sub/my_sub_lib.h' is invalid because "
+ "ERROR /workspace/test/BUILD:1:1: Label '//test:sub/my_sub_lib.h' is invalid because "
+ "'test/sub' is a subpackage; perhaps you meant to put the colon here: "
+ "'//test/sub:my_sub_lib.h'?");
}
@@ -319,7 +319,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:skyrule");
assertContainsEvent(
- "ERROR /workspace/test/BUILD:3:10: Label '//test:sub/my_sub_lib.h' is invalid because "
+ "ERROR /workspace/test/BUILD:2:1: Label '//test:sub/my_sub_lib.h' is invalid because "
+ "'test/sub' is a subpackage; perhaps you meant to put the colon here: "
+ "'//test/sub:my_sub_lib.h'?");
}
@@ -368,7 +368,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//:cclib");
assertContainsEvent(
- "/workspace/BUILD:2:10: Label '//:r/my_sub_lib.h' is invalid because "
+ "/workspace/BUILD:1:1: Label '//:r/my_sub_lib.h' is invalid because "
+ "'@r//' is a subpackage");
}
@@ -387,7 +387,7 @@
reporter.removeHandler(failFastHandler);
getConfiguredTarget("@r//:cclib");
assertContainsEvent(
- "/external/r/BUILD:2:10: Label '@r//:sub/my_sub_lib.h' is invalid because "
+ "/external/r/BUILD:1:1: Label '@r//:sub/my_sub_lib.h' is invalid because "
+ "'@r//sub' is a subpackage; perhaps you meant to put the colon here: "
+ "'@r//sub:my_sub_lib.h'?");
}
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java b/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java
index 69f5414..102f88c 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/PackageFactoryBuilderFactoryForBazelUnitTests.java
@@ -56,7 +56,6 @@
: Package.Builder.DefaultHelper.INSTANCE;
return new PackageFactory(
ruleClassProvider,
- attributeContainerFactory,
environmentExtensions,
version,
packageBuilderHelperForTesting);
