Skip to content

Commit bd0bddc

Browse files
committed
[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
1 parent 68099b1 commit bd0bddc

File tree

7 files changed

+22
-39
lines changed

7 files changed

+22
-39
lines changed

llvm/lib/Support/CommandLine.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,21 +1737,6 @@ bool Option::addOccurrence(unsigned pos, StringRef ArgName, StringRef Value,
17371737
if (!MultiArg)
17381738
NumOccurrences++; // Increment the number of times we have been seen
17391739

1740-
switch (getNumOccurrencesFlag()) {
1741-
case Optional:
1742-
if (NumOccurrences > 1)
1743-
return error("may only occur zero or one times!", ArgName);
1744-
break;
1745-
case Required:
1746-
if (NumOccurrences > 1)
1747-
return error("must occur exactly one time!", ArgName);
1748-
LLVM_FALLTHROUGH;
1749-
case OneOrMore:
1750-
case ZeroOrMore:
1751-
case ConsumeAfter:
1752-
break;
1753-
}
1754-
17551740
return handleOccurrence(pos, ArgName, Value);
17561741
}
17571742

llvm/test/tools/llvm-libtool-darwin/create-static-lib.test

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,10 @@
44
# RUN: yaml2obj %S/Inputs/input2.yaml -o %t-input2.o
55
# RUN: llvm-as %S/Inputs/x86_64-osx.ll -o %t-x86_64.bc
66

7-
# RUN: rm -rf %t.lib
7+
# RUN: rm -rf %t.lib %t2.lib
88
# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o %t-input2.o %t-x86_64.bc
9+
# RUN: llvm-libtool-darwin -static -o %t2.lib -o %t.lib %t-input1.o %t-input2.o %t-x86_64.bc
10+
# RUN: not ls %t2.lib
911

1012
## Check that binaries are present:
1113
# RUN: llvm-ar t %t.lib | \

llvm/test/tools/llvm-libtool-darwin/deterministic-library.test

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,12 @@
2424
# CHECK-NONDETERMINISTIC: {{[[:space:]]1995[[:space:]]}}
2525

2626
## D Flag specified more than once:
27-
# RUN: not llvm-libtool-darwin -static -o %t.lib %t-input1.o -D -D 2>&1 | \
28-
# RUN: FileCheck %s --check-prefix=CHECK-ERROR-D
29-
30-
# CHECK-ERROR-D: for the -D option: may only occur zero or one times!
27+
# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o -D -D 2>&1 | count 0
28+
# RUN: env TZ=GMT llvm-ar tv %t.lib | FileCheck %s --check-prefix=CHECK-DETERMINISTIC
3129

3230
## U Flag specified more than once:
33-
# RUN: not llvm-libtool-darwin -static -o %t.lib %t-input1.o -U -U 2>&1 | \
34-
# RUN: FileCheck %s --check-prefix=CHECK-ERROR-U
35-
36-
# CHECK-ERROR-U: for the -U option: may only occur zero or one times!
31+
# RUN: llvm-libtool-darwin -static -o %t.lib %t-input1.o -U -U 2>&1 | count 0
32+
# RUN: env TZ=GMT llvm-ar tv %t.lib | FileCheck %s --check-prefix=CHECK-NONDETERMINISTIC
3733

3834
## Both D and U flags specified:
3935
# RUN: not llvm-libtool-darwin -static -o %t.lib %t-input1.o -D -U 2>&1 | \

llvm/test/tools/llvm-libtool-darwin/filelist.test

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@
3030

3131
# RUN: rm -rf %t/dirname && mkdir -p %t/dirname
3232
# RUN: yaml2obj %S/Inputs/input1.yaml -o %t/dirname/%basename_t.tmp-input1.o
33-
# RUN: echo %basename_t.tmp-input1.o > %t.files.txt
33+
# RUN: echo %basename_t.tmp-input1.o > %t.files2.txt
3434

3535
## Passing in dirname:
36-
# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files.txt,%t/dirname
36+
# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files2.txt,%t/dirname
3737
# RUN: llvm-ar t %t.lib | \
3838
# RUN: FileCheck %s --check-prefix=DIRNAME-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp
3939
# RUN: llvm-nm --print-armap %t.lib | \
@@ -46,7 +46,7 @@
4646
# DIRNAME-SYMBOLS-EMPTY:
4747

4848
## Passing both -filelist option and object file as input:
49-
# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files.txt,%t/dirname %t-input2.o
49+
# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.files2.txt,%t/dirname %t-input2.o
5050
# RUN: llvm-ar t %t.lib | \
5151
# RUN: FileCheck %s --check-prefix=REVERSE-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp
5252
# RUN: llvm-nm --print-armap %t.lib | \
@@ -106,7 +106,6 @@
106106

107107
## Filelist option specified more than once:
108108
# RUN: touch %t.list1.txt and %t.list2.txt
109-
# RUN: not llvm-libtool-darwin -static -o %t.lib -filelist %t.list1.txt -filelist %t.list2.txt 2>&1 | \
110-
# RUN: FileCheck %s --check-prefix=DUPLICATE-ERROR
111-
112-
# DUPLICATE-ERROR: for the --filelist option: may only occur zero or one times!
109+
# RUN: llvm-libtool-darwin -static -o %t.lib -filelist %t.empty-list -filelist %t.files.txt 2>&1
110+
# RUN: llvm-ar t %t.lib | \
111+
# RUN: FileCheck %s --check-prefix=CHECK-NAMES --implicit-check-not={{.}} -DPREFIX=%basename_t.tmp

llvm/test/tools/llvm-libtool-darwin/invalid-input-output-args.test

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,6 @@
1818

1919
# MISSING: for the -o option: requires a value!
2020

21-
## Passing in two output files:
22-
# RUN: not llvm-libtool-darwin -static %t.input -o %t.lib1 -o %t.lib2 2>&1 | \
23-
# RUN: FileCheck %s --check-prefix=DOUBLE-OUTPUT
24-
25-
# DOUBLE-OUTPUT: for the -o option: may only occur zero or one times!
26-
2721
## Input file not found:
2822
# RUN: not llvm-libtool-darwin -static -o %t.lib %t.missing 2>&1 | \
2923
# RUN: FileCheck %s --check-prefix=NO-FILE -DFILE=%t.missing -DMSG=%errc_ENOENT

llvm/unittests/Support/CommandLineTest.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,14 @@ TEST(CommandLineTest, HideUnrelatedOptionsMulti) {
420420
<< "Hid default option that should be visable.";
421421
}
422422

423+
TEST(CommandLineTest, SetMultiValues) {
424+
StackOption<int> Option("option");
425+
const char *args[] = {"prog", "-option=1", "-option=2"};
426+
EXPECT_TRUE(cl::ParseCommandLineOptions(array_lengthof(args), args,
427+
StringRef(), &llvm::nulls()));
428+
EXPECT_EQ(Option, 2);
429+
}
430+
423431
TEST(CommandLineTest, SetValueInSubcategories) {
424432
cl::ResetCommandLineParser();
425433

mlir/test/Pass/pipeline-options-parsing.mlir

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// RUN: not mlir-opt %s -pass-pipeline='builtin.module(test-module-pass{test-option=3})' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_2 %s
33
// 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
44
// RUN: not mlir-opt %s -pass-pipeline='test-options-pass{list=3 list=notaninteger}' 2>&1 | FileCheck --check-prefix=CHECK_ERROR_4 %s
5-
// 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
5+
// RUN: mlir-opt %s -pass-pipeline='builtin.func(test-options-pass{list=1,2,3,4 list=5 string=value1 string=value2})'
66
// 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
77
// 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
88
// 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 @@
1111
// CHECK_ERROR_2: no such option test-option
1212
// CHECK_ERROR_3: no such option invalid-option
1313
// CHECK_ERROR_4: 'notaninteger' value invalid for integer argument
14-
// CHECK_ERROR_5: string option: may only occur zero or one times
1514

1615
// 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}
1716
// CHECK_2: test-options-pass{list=1 string= string-list=a,b}

0 commit comments

Comments
 (0)