[llvm] bd0bddc - [CommandLine] Remove `may only occur zero or one times!` error
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 11 11:25:11 PST 2022
Author: Fangrui Song
Date: 2022-03-11T11:25:04-08:00
New Revision: bd0bddc1ea72183813d49508c8b4e73920869ea5
URL: https://github.com/llvm/llvm-project/commit/bd0bddc1ea72183813d49508c8b4e73920869ea5
DIFF: https://github.com/llvm/llvm-project/commit/bd0bddc1ea72183813d49508c8b4e73920869ea5.diff
LOG: [CommandLine] Remove `may only occur zero or one times!` error
Early adoption of new technologies or adjusting certain code generation/IR optimization thresholds
is often available through some cl::opt options (which have unstable surfaces).
Specifying such an option twice will lead to an error.
```
% clang -c a.c -mllvm -disable-binop-extract-shuffle -mllvm -disable-binop-extract-shuffle
clang (LLVM option parsing): for the --disable-binop-extract-shuffle option: may only occur zero or one times!
% clang -c a.c -mllvm -hwasan-instrument-reads=0 -mllvm -hwasan-instrument-reads=0
clang (LLVM option parsing): for the --hwasan-instrument-reads option: may only occur zero or one times!
% clang -c a.c -mllvm --scalar-evolution-max-arith-depth=32 -mllvm --scalar-evolution-max-arith-depth=16
clang (LLVM option parsing): for the --scalar-evolution-max-arith-depth option: may only occur zero or one times!
```
The option is specified twice, because there is sometimes a global setting and
a specific file or project may need to override (or duplicately specify) the
value.
The error is contrary to the common practice of getopt/getopt_long command line
utilities that let the last option win and the `getLastArg` behavior used by
Clang driver options. I have seen such errors for several times. I think the
error just makes users inconvenient, while providing very little value on
discouraging production usage of unstable surfaces (this goal is itself
controversial, because developers might not want to commit to a stable surface
too early, or there is just some subtle codegen toggle which is infeasible to
have a driver option). Therefore, I suggest we drop the diagnostic, at least
before the diagnostic gets sufficiently better support for the overridding needs.
Removing the error is a degraded error checking experience. I think this error
checking behavior, if desirable, should be enabled explicitly by tools. Users
preferring the behavior can figure out a way to do so.
Reviewed By: jhenderson, rnk
Differential Revision: https://reviews.llvm.org/D120455
Added:
Modified:
llvm/lib/Support/CommandLine.cpp
llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
llvm/test/tools/llvm-libtool-darwin/deterministic-library.test
llvm/test/tools/llvm-libtool-darwin/filelist.test
llvm/test/tools/llvm-libtool-darwin/invalid-input-output-args.test
llvm/unittests/Support/CommandLineTest.cpp
mlir/test/Pass/pipeline-options-parsing.mlir
Removed:
################################################################################
diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp
index fd9022caefb7f..4c9250236a3b5 100644
--- a/llvm/lib/Support/CommandLine.cpp
+++ b/llvm/lib/Support/CommandLine.cpp
@@ -1737,21 +1737,6 @@ bool Option::addOccurrence(unsigned pos, StringRef ArgName, StringRef Value,
if (!MultiArg)
NumOccurrences++; // Increment the number of times we have been seen
- switch (getNumOccurrencesFlag()) {
- case Optional:
- if (NumOccurrences > 1)
- return error("may only occur zero or one times!", ArgName);
- break;
- case Required:
- if (NumOccurrences > 1)
- return error("must occur exactly one time!", ArgName);
- LLVM_FALLTHROUGH;
- case OneOrMore:
- case ZeroOrMore:
- case ConsumeAfter:
- break;
- }
-
return handleOccurrence(pos, ArgName, Value);
}
diff --git a/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test b/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
index 8453e786e4ccf..73a34a191bcec 100644
--- a/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
+++ b/llvm/test/tools/llvm-libtool-darwin/create-static-lib.test
@@ -4,8 +4,10 @@
# RUN: yaml2obj %S/Inputs/input2.yaml -o %t-input2.o
# RUN: llvm-as %S/Inputs/x86_64-osx.ll -o %t-x86_64.bc
-# RUN: rm -rf %t.lib
+# RUN: rm -rf %t.lib %t2.lib
# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o %t-input2.o %t-x86_64.bc
+# RUN: llvm-libtool-darwin -static -o %t2.lib -o %t.lib %t-input1.o %t-input2.o %t-x86_64.bc
+# RUN: not ls %t2.lib
## Check that binaries are present:
# RUN: llvm-ar t %t.lib | \
diff --git a/llvm/test/tools/llvm-libtool-darwin/deterministic-library.test b/llvm/test/tools/llvm-libtool-darwin/deterministic-library.test
index 5408309f6e958..d3923afb29804 100644
--- a/llvm/test/tools/llvm-libtool-darwin/deterministic-library.test
+++ b/llvm/test/tools/llvm-libtool-darwin/deterministic-library.test
@@ -24,16 +24,12 @@
# CHECK-NONDETERMINISTIC: {{[[:space:]]1995[[:space:]]}}
## D Flag specified more than once:
-# RUN: not llvm-libtool-darwin -static -o %t.lib %t-input1.o -D -D 2>&1 | \
-# RUN: FileCheck %s --check-prefix=CHECK-ERROR-D
-
-# CHECK-ERROR-D: for the -D option: may only occur zero or one times!
+# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o -D -D 2>&1 | count 0
+# RUN: env TZ=GMT llvm-ar tv %t.lib | FileCheck %s --check-prefix=CHECK-DETERMINISTIC
## U Flag specified more than once:
-# RUN: not llvm-libtool-darwin -static -o %t.lib %t-input1.o -U -U 2>&1 | \
-# RUN: FileCheck %s --check-prefix=CHECK-ERROR-U
-
-# CHECK-ERROR-U: for the -U option: may only occur zero or one times!
+# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o -U -U 2>&1 | count 0
+# RUN: env TZ=GMT llvm-ar tv %t.lib | FileCheck %s --check-prefix=CHECK-NONDETERMINISTIC
## Both D and U flags specified:
# RUN: not llvm-libtool-darwin -static -o %t.lib %t-input1.o -D -U 2>&1 | \
diff --git a/llvm/test/tools/llvm-libtool-darwin/filelist.test b/llvm/test/tools/llvm-libtool-darwin/filelist.test
index b6e95383c4677..1c1da31facaf3 100644
--- a/llvm/test/tools/llvm-libtool-darwin/filelist.test
+++ b/llvm/test/tools/llvm-libtool-darwin/filelist.test
@@ -30,10 +30,10 @@
# RUN: rm -rf %t/dirname && mkdir -p %t/dirname
# RUN: yaml2obj %S/Inputs/input1.yaml -o %t/dirname/%basename_t.tmp-input1.o
-# RUN: echo %basename_t.tmp-input1.o > %t.files.txt
+# RUN: echo %basename_t.tmp-input1.o > %t.files2.txt
## Passing in dirname:
-# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files.txt,%t/dirname
+# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files2.txt,%t/dirname
# RUN: llvm-ar t %t.lib | \
# RUN: FileCheck %s --check-prefix=DIRNAME-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp
# RUN: llvm-nm --print-armap %t.lib | \
@@ -46,7 +46,7 @@
# DIRNAME-SYMBOLS-EMPTY:
## Passing both -filelist option and object file as input:
-# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files.txt,%t/dirname %t-input2.o
+# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files2.txt,%t/dirname %t-input2.o
# RUN: llvm-ar t %t.lib | \
# RUN: FileCheck %s --check-prefix=REVERSE-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp
# RUN: llvm-nm --print-armap %t.lib | \
@@ -106,7 +106,6 @@
## Filelist option specified more than once:
# RUN: touch %t.list1.txt and %t.list2.txt
-# RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.list1.txt -filelist %t.list2.txt 2>&1 | \
-# RUN: FileCheck %s --check-prefix=DUPLICATE-ERROR
-
-# DUPLICATE-ERROR: for the --filelist option: may only occur zero or one times!
+# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.empty-list -filelist %t.files.txt 2>&1
+# RUN: llvm-ar t %t.lib | \
+# RUN: FileCheck %s --check-prefix=CHECK-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp
diff --git a/llvm/test/tools/llvm-libtool-darwin/invalid-input-output-args.test b/llvm/test/tools/llvm-libtool-darwin/invalid-input-output-args.test
index 2a6d841475b16..1caaa3f292036 100644
--- a/llvm/test/tools/llvm-libtool-darwin/invalid-input-output-args.test
+++ b/llvm/test/tools/llvm-libtool-darwin/invalid-input-output-args.test
@@ -18,12 +18,6 @@
# MISSING: for the -o option: requires a value!
-## Passing in two output files:
-# RUN: not llvm-libtool-darwin -static %t.input -o %t.lib1 -o %t.lib2 2>&1 | \
-# RUN: FileCheck %s --check-prefix=DOUBLE-OUTPUT
-
-# DOUBLE-OUTPUT: for the -o option: may only occur zero or one times!
-
## Input file not found:
# RUN: not llvm-libtool-darwin -static -o %t.lib %t.missing 2>&1 | \
# RUN: FileCheck %s --check-prefix=NO-FILE -DFILE=%t.missing -DMSG=%errc_ENOENT
diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index 7fe1cf46ce260..e68761eca3525 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -420,6 +420,14 @@ TEST(CommandLineTest, HideUnrelatedOptionsMulti) {
<< "Hid default option that should be visable.";
}
+TEST(CommandLineTest, SetMultiValues) {
+ StackOption<int> Option("option");
+ const char *args[] = {"prog", "-option=1", "-option=2"};
+ EXPECT_TRUE(cl::ParseCommandLineOptions(array_lengthof(args), args,
+ StringRef(), &llvm::nulls()));
+ EXPECT_EQ(Option, 2);
+}
+
TEST(CommandLineTest, SetValueInSubcategories) {
cl::ResetCommandLineParser();
diff --git a/mlir/test/Pass/pipeline-options-parsing.mlir b/mlir/test/Pass/pipeline-options-parsing.mlir
index ab0e482373ff7..53a5712d79f41 100644
--- a/mlir/test/Pass/pipeline-options-parsing.mlir
+++ b/mlir/test/Pass/pipeline-options-parsing.mlir
@@ -2,7 +2,7 @@
// RUN: not mlir-opt %s -pass-pipeline='builtin.module(test-module-pass{test-option=3})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_2 %s
// RUN: not mlir-opt %s -pass-pipeline='builtin.module(builtin.func(test-options-pass{list=3}), test-module-pass{invalid-option=3})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_3 %s
// RUN: not mlir-opt %s -pass-pipeline='test-options-pass{list=3 list=notaninteger}' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_4 %s
-// RUN: not mlir-opt %s -pass-pipeline='builtin.func(test-options-pass{list=1,2,3,4 list=5 string=value1 string=value2})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_5 %s
+// RUN: mlir-opt %s -pass-pipeline='builtin.func(test-options-pass{list=1,2,3,4 list=5 string=value1 string=value2})'
// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.func(test-options-pass{string-list=a list=1,2,3,4 string-list=b,c list=5 string-list=d string=nested_pipeline{arg1=10 arg2=" {} " arg3=true}})' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_1 %s
// RUN: mlir-opt %s -verify-each=false -test-options-pass-pipeline='list=1 string-list=a,b' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_2 %s
// RUN: mlir-opt %s -verify-each=false -pass-pipeline='builtin.module(builtin.func(test-options-pass{list=3}), builtin.func(test-options-pass{list=1,2,3,4}))' -test-dump-pipeline 2>&1 | FileCheck --check-prefix=CHECK_3 %s
@@ -11,7 +11,6 @@
// CHECK_ERROR_2: no such option test-option
// CHECK_ERROR_3: no such option invalid-option
// CHECK_ERROR_4: 'notaninteger' value invalid for integer argument
-// CHECK_ERROR_5: string option: may only occur zero or one times
// CHECK_1: test-options-pass{list=1,2,3,4,5 string=nested_pipeline{arg1=10 arg2=" {} " arg3=true} string-list=a,b,c,d}
// CHECK_2: test-options-pass{list=1 string= string-list=a,b}
More information about the llvm-commits
mailing list