Remove the --incompatible_disallow_old_octal_notation flag
It is not useful to keep the flag around.
https://github.com/bazelbuild/bazel/issues/8059
RELNOTES: None.
PiperOrigin-RevId: 249537925
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index edcfbf3..f1ee3b4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -359,21 +359,6 @@
+ "instead return a list of provider instances.")
public boolean incompatibleDisallowStructProviderSyntax;
- @Option(
- name = "incompatible_disallow_old_octal_notation",
- defaultValue = "true",
- category = "incompatible changes",
- documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
- effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
- metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
- },
- help =
- "If set to true, octal numbers like `0123` are forbidden, they should be written "
- + "`0o123` instead. See https://github.com/bazelbuild/bazel/issues/8059")
- public boolean incompatibleDisallowOldOctalNotation;
-
/** Controls legacy arguments to ctx.actions.Args#add. */
@Option(
name = "incompatible_disallow_old_style_args_add",
@@ -634,7 +619,6 @@
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(
incompatibleDisallowLoadLabelsToCrossPackageBoundaries)
.incompatibleDisallowNativeInBuildFile(incompatibleDisallowNativeInBuildFile)
- .incompatibleDisallowOldOctalNotation(incompatibleDisallowOldOctalNotation)
.incompatibleDisallowOldStyleArgsAdd(incompatibleDisallowOldStyleArgsAdd)
.incompatibleDisallowStructProviderSyntax(incompatibleDisallowStructProviderSyntax)
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index fe334f1..da2ee67 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -541,7 +541,6 @@
public static void execAndExport(BuildFileAST ast, Label extensionLabel,
EventHandler eventHandler,
com.google.devtools.build.lib.syntax.Environment extensionEnv) throws InterruptedException {
- ast.replayLexerEvents(extensionEnv, eventHandler);
ImmutableList<Statement> statements = ast.getStatements();
for (Statement statement : statements) {
ast.execTopLevelStatement(statement, extensionEnv, eventHandler);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index 47beaf1..8902c8d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -49,8 +49,6 @@
*/
private final boolean containsErrors;
- private final List<Event> octalEvents;
-
@Nullable private final String contentHashCode;
private BuildFileAST(
@@ -59,15 +57,13 @@
String contentHashCode,
Location location,
ImmutableList<Comment> comments,
- @Nullable ImmutableList<SkylarkImport> imports,
- List<Event> octalEvents) {
+ @Nullable ImmutableList<SkylarkImport> imports) {
this.statements = statements;
this.containsErrors = containsErrors;
this.contentHashCode = contentHashCode;
this.comments = comments;
this.setLocation(location);
this.imports = imports;
- this.octalEvents = octalEvents;
}
private static BuildFileAST create(
@@ -102,8 +98,7 @@
contentHashCode,
result.location,
ImmutableList.copyOf(result.comments),
- skylarkImports.second,
- result.octalEvents);
+ skylarkImports.second);
}
private static BuildFileAST create(
@@ -140,8 +135,7 @@
null,
this.statements.get(firstStatement).getLocation(),
ImmutableList.of(),
- imports.build(),
- octalEvents);
+ imports.build());
}
/**
@@ -208,15 +202,6 @@
return imports.build();
}
- /** Returns true if there was no error event. */
- public boolean replayLexerEvents(Environment env, EventHandler eventHandler) {
- if (env.getSemantics().incompatibleDisallowOldOctalNotation() && !octalEvents.isEmpty()) {
- Event.replayEventsOn(eventHandler, octalEvents);
- return false;
- }
- return true;
- }
-
/**
* Executes this build file in a given Environment.
*
@@ -235,9 +220,6 @@
*/
public boolean exec(Environment env, EventHandler eventHandler) throws InterruptedException {
boolean ok = true;
- if (!replayLexerEvents(env, eventHandler)) {
- return false;
- }
for (Statement stmt : statements) {
if (!execTopLevelStatement(stmt, env, eventHandler)) {
ok = false;
@@ -389,8 +371,7 @@
/* contentHashCode= */ null,
result.location,
ImmutableList.copyOf(result.comments),
- /* imports= */ null,
- result.octalEvents);
+ /* imports= */ null);
}
/**
@@ -403,8 +384,7 @@
if (valid || containsErrors) {
return this;
}
- return new BuildFileAST(
- statements, true, contentHashCode, getLocation(), comments, imports, octalEvents);
+ return new BuildFileAST(statements, true, contentHashCode, getLocation(), comments, imports);
}
public static BuildFileAST parseString(EventHandler eventHandler, String... content) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
index f098a26..c5a5c1a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Lexer.java
@@ -104,14 +104,6 @@
private int dents; // number of saved INDENT (>0) or OUTDENT (<0) tokens to return
/**
- * OctalEvents contains the errors related to the old octal notation (0123 instead of 0o123). This
- * is not handled by the normal eventHandler. Instead, it is passed to the parser and then the
- * AST. During the evaluation, we can decide to show the events based on a flag in
- * StarlarkSemantics. This code is temporary, during the migration.
- */
- private List<Event> octalEvents = new ArrayList<>();
-
- /**
* Constructs a lexer which tokenizes the contents of the specified InputBuffer. Any errors during
* lexing are reported on "handler".
*/
@@ -137,10 +129,6 @@
return comments;
}
- List<Event> getOctalEvents() {
- return octalEvents;
- }
-
/**
* Returns the filename from which the lexer's input came. Returns an empty value if the input
* came from a string.
@@ -701,10 +689,7 @@
} else if (literal.startsWith("0") && literal.length() > 1) {
radix = 8;
substring = literal.substring(1);
- octalEvents.add(
- Event.error(
- createLocation(oldPos, pos),
- "Invalid octal value `" + literal + "`, should be: `0o" + substring + "`."));
+ error("invalid octal value `" + literal + "`, should be: `0o" + substring + "`");
} else {
radix = 10;
substring = literal;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index cec994e..497a526 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -62,21 +62,17 @@
/** Whether the file contained any errors. */
public final boolean containsErrors;
- public final List<Event> octalEvents;
-
public ParseResult(
List<Statement> statements,
List<Comment> comments,
Location location,
- boolean containsErrors,
- List<Event> octalEvents) {
+ boolean containsErrors) {
// No need to copy here; when the object is created, the parser instance is just about to go
// out of scope and be garbage collected.
this.statements = Preconditions.checkNotNull(statements);
this.comments = Preconditions.checkNotNull(comments);
this.location = location;
this.containsErrors = containsErrors;
- this.octalEvents = octalEvents;
}
}
@@ -209,11 +205,7 @@
}
boolean errors = parser.errorsCount > 0 || lexer.containsErrors();
return new ParseResult(
- statements,
- lexer.getComments(),
- locationFromStatements(lexer, statements),
- errors,
- lexer.getOctalEvents());
+ statements, lexer.getComments(), locationFromStatements(lexer, statements), errors);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 3d1d3e6..8a58b3d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -157,8 +157,6 @@
public abstract boolean incompatibleDisallowNativeInBuildFile();
- public abstract boolean incompatibleDisallowOldOctalNotation();
-
public abstract boolean incompatibleDisallowOldStyleArgsAdd();
public abstract boolean incompatibleDisallowRuleExecutionPlatformConstraintsAllowed();
@@ -234,7 +232,6 @@
.incompatibleDisallowLegacyJavaInfo(false)
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(true)
.incompatibleDisallowNativeInBuildFile(true)
- .incompatibleDisallowOldOctalNotation(true)
.incompatibleDisallowOldStyleArgsAdd(true)
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(false)
.incompatibleDisallowStructProviderSyntax(false)
@@ -300,8 +297,6 @@
public abstract Builder incompatibleDisallowLoadLabelsToCrossPackageBoundaries(boolean value);
- public abstract Builder incompatibleDisallowOldOctalNotation(boolean value);
-
public abstract Builder incompatibleDisallowOldStyleArgsAdd(boolean value);
public abstract Builder incompatibleDisallowNativeInBuildFile(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 7a550db..d202f70 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -145,7 +145,6 @@
"--incompatible_disallow_legacy_java_provider=" + rand.nextBoolean(),
"--incompatible_disallow_load_labels_to_cross_package_boundaries=" + rand.nextBoolean(),
"--incompatible_disallow_native_in_build_file=" + rand.nextBoolean(),
- "--incompatible_disallow_old_octal_notation=" + rand.nextBoolean(),
"--incompatible_disallow_old_style_args_add=" + rand.nextBoolean(),
"--incompatible_disallow_struct_provider_syntax=" + rand.nextBoolean(),
"--incompatible_disallow_rule_execution_platform_constraints_allowed=" + rand.nextBoolean(),
@@ -197,7 +196,6 @@
.incompatibleDisallowLegacyJavaProvider(rand.nextBoolean())
.incompatibleDisallowLoadLabelsToCrossPackageBoundaries(rand.nextBoolean())
.incompatibleDisallowNativeInBuildFile(rand.nextBoolean())
- .incompatibleDisallowOldOctalNotation(rand.nextBoolean())
.incompatibleDisallowOldStyleArgsAdd(rand.nextBoolean())
.incompatibleDisallowStructProviderSyntax(rand.nextBoolean())
.incompatibleDisallowRuleExecutionPlatformConstraintsAllowed(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index c0b6ca4..89b7ea1 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -2977,28 +2977,4 @@
+ "//test/skylark:ext3.bzl, //test/skylark:ext4.bzl]");
}
}
-
- @Test
- public void testOldOctalNotationIsForbidden() throws Exception {
- setSkylarkSemanticsOptions("--incompatible_disallow_old_octal_notation=true");
-
- scratch.file("test/extension.bzl", "y = 0246");
-
- scratch.file("test/BUILD", "load('//test:extension.bzl', 'y')", "cc_library(name = 'r')");
-
- reporter.removeHandler(failFastHandler);
- getConfiguredTarget("//test:r");
- assertContainsEvent("Invalid octal value `0246`, should be: `0o246`");
- }
-
- @Test
- public void testOldOctalNotation() throws Exception {
- setSkylarkSemanticsOptions("--incompatible_disallow_old_octal_notation=false");
-
- scratch.file("test/extension.bzl", "y = 0246");
-
- scratch.file("test/BUILD", "load('//test:extension.bzl', 'y')", "cc_library(name = 'r')");
-
- getConfiguredTarget("//test:r");
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java b/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java
index 801e5e1..7226d1c 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/LexerTest.java
@@ -205,7 +205,6 @@
assertThat(values(tokens("12345-"))).isEqualTo("INT(12345) MINUS NEWLINE EOF");
// octal
- assertThat(values(tokens("012345-"))).isEqualTo("INT(5349) MINUS NEWLINE EOF");
assertThat(values(tokens("0o12345-"))).isEqualTo("INT(5349) MINUS NEWLINE EOF");
assertThat(values(tokens("0O77"))).isEqualTo("INT(63) NEWLINE EOF");
@@ -218,6 +217,10 @@
assertThat(lastError.toString())
.isEqualTo("/some/path.txt:1: invalid base-8 integer constant: 0o");
+ assertThat(values(tokens("012345"))).isEqualTo("INT(5349) NEWLINE EOF");
+ assertThat(lastError.toString())
+ .isEqualTo("/some/path.txt:1: invalid octal value `012345`, should be: `0o12345`");
+
// hexadecimal (uppercase)
assertThat(values(tokens("0X12345F-"))).isEqualTo("INT(1193055) MINUS NEWLINE EOF");
