Skip to content

Commit f1b8ea1

Browse files
akyrtziakyrtzi
committed
[clang/cas] Switch -fmodule-file-cache-key option to being MultiArg
Previously parsing for the option assumes that `=` cannot appear in the CASID string, which isn’t a good assumption because `=` is a common character in `base64` strings. Switch to `MultiArg` option style which eliminates the issue of what string separator to use. (cherry picked from commit c4c7669)
1 parent bf76389 commit f1b8ea1

12 files changed

+92
-38
lines changed

‎clang/include/clang/Basic/DiagnosticFrontendKinds.td

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,6 @@ def warn_alias_with_section : Warning<
323323
"as the %select{aliasee|resolver}2">,
324324
InGroup<IgnoredAttributes>;
325325

326-
def err_module_cache_key_spelling : Error<
327-
"option '-fmodule-file-cache-key' should be of the form <path>=<key>">;
328-
329326
let CategoryName = "Instrumentation Issue" in {
330327
def warn_profile_data_out_of_date : Warning<
331328
"profile data may be out of date: of %0 function%s0, %1 %plural{1:has|:have}1"

‎clang/include/clang/Driver/Options.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5942,8 +5942,8 @@ def ftest_module_file_extension_EQ :
59425942
Joined<["-"], "ftest-module-file-extension=">,
59435943
HelpText<"introduce a module file extension for testing purposes. "
59445944
"The argument is parsed as blockname:major:minor:hashed:user info">;
5945-
def fmodule_file_cache_key : Joined<["-"], "fmodule-file-cache-key=">,
5946-
MetaVarName<"<path>=<key>">,
5945+
def fmodule_file_cache_key : MultiArg<["-"], "fmodule-file-cache-key", 2>,
5946+
MetaVarName<"<path> <key>">,
59475947
HelpText<"Make the module with the given compile job cache key available as "
59485948
"if it were at <path>. This option may be combined with "
59495949
"-fmodule-file= to import the module. The module must have "

‎clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,18 @@ static void GenerateArg(SmallVectorImpl<const char *> &Args,
663663
Opt.getKind(), 0, Value);
664664
}
665665

666+
static void GenerateMultiArg(SmallVectorImpl<const char *> &Args,
667+
llvm::opt::OptSpecifier OptSpecifier,
668+
ArrayRef<StringRef> Values,
669+
CompilerInvocation::StringAllocator SA) {
670+
Option Opt = getDriverOptTable().getOption(OptSpecifier);
671+
assert(Opt.getKind() == Option::MultiArgClass);
672+
assert(Opt.getNumArgs() == Values.size());
673+
Args.push_back(SA(Opt.getPrefix() + Opt.getName()));
674+
for (StringRef Value : Values)
675+
Args.push_back(SA(Value));
676+
}
677+
666678
// Parse command line arguments into CompilerInvocation.
667679
using ParseFn =
668680
llvm::function_ref<bool(CompilerInvocation &, ArrayRef<const char *>,
@@ -2921,7 +2933,7 @@ static void GenerateFrontendArgs(const FrontendOptions &Opts,
29212933
for (const auto &ModuleFile : Opts.ModuleFiles)
29222934
GenerateArg(Args, OPT_fmodule_file, ModuleFile, SA);
29232935
for (const auto &A : Opts.ModuleCacheKeys)
2924-
GenerateArg(Args, OPT_fmodule_file_cache_key, A.first + "=" + A.second, SA);
2936+
GenerateMultiArg(Args, OPT_fmodule_file_cache_key, {A.first, A.second}, SA);
29252937

29262938
if (Opts.AuxTargetCPU)
29272939
GenerateArg(Args, OPT_aux_target_cpu, *Opts.AuxTargetCPU, SA);
@@ -3157,12 +3169,9 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
31573169
Opts.ModuleFiles.push_back(std::string(Val));
31583170
}
31593171
for (const Arg *A : Args.filtered(OPT_fmodule_file_cache_key)) {
3160-
auto [Path, Key] = StringRef(A->getValue()).rsplit('=');
3161-
if (Key.empty() || Path.empty()) {
3162-
Diags.Report(diag::err_module_cache_key_spelling);
3163-
} else {
3164-
Opts.ModuleCacheKeys.emplace_back(std::string(Path), std::string(Key));
3165-
}
3172+
ArrayRef<const char *> Values = A->getValues();
3173+
assert(Values.size() == 2);
3174+
Opts.ModuleCacheKeys.emplace_back(Values[0], Values[1]);
31663175
}
31673176

31683177
if (Opts.ProgramAction != frontend::GenerateModule && Opts.IsSystemModule)

‎clang/test/CAS/fmodule-file-cache-key-errors.c

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,39 @@
1616
// RUN: -fcache-compile-job -Rcompile-job-cache &> %t/invalid.txt
1717
// RUN: cat %t/invalid.txt | FileCheck %s -check-prefix=INVALID
1818

19-
// INVALID: error: option '-fmodule-file-cache-key' should be of the form <path>=<key>
19+
// INVALID: error: unknown argument: '-fmodule-file-cache-key=INVALID'
2020

2121
// RUN: not %clang_cc1 -triple x86_64-apple-macos11 \
2222
// RUN: -fmodules -fno-implicit-modules \
23-
// RUN: -fmodule-file-cache-key=PATH=KEY \
23+
// RUN: -fmodule-file-cache-key INVALID \
24+
// RUN: -fsyntax-only %t/tu.c \
25+
// RUN: -fcas-path %t.cas -fcas-fs @%t/casid \
26+
// RUN: -fcache-compile-job -Rcompile-job-cache &> %t/invalid2.txt
27+
// RUN: FileCheck %s -check-prefix=INVALID2 -input-file=%t/invalid2.txt
28+
29+
// INVALID2: error: CAS cannot load module with key '-fsyntax-only' from -fmodule-file-cache-key
30+
31+
// RUN: not %clang_cc1 -triple x86_64-apple-macos11 \
32+
// RUN: -fmodules -fno-implicit-modules \
33+
// RUN: -fmodule-file-cache-key INVALID ALSO_INVALID MORE_INVALID \
34+
// RUN: -fsyntax-only %t/tu.c \
35+
// RUN: -fcas-path %t.cas -fcas-fs @%t/casid \
36+
// RUN: -fcache-compile-job -Rcompile-job-cache &> %t/invalid3.txt
37+
// RUN: FileCheck %s -check-prefix=INVALID3 -input-file=%t/invalid3.txt
38+
39+
// INVALID3: error: error reading 'MORE_INVALID'
40+
41+
// RUN: not %clang_cc1 -triple x86_64-apple-macos11 \
42+
// RUN: -fmodules -fno-implicit-modules \
43+
// RUN: -fmodule-file-cache-key PATH KEY \
2444
// RUN: -fsyntax-only %t/tu.c \
2545
// RUN: -fcas-path %t.cas -fcas-fs @%t/casid \
2646
// RUN: -fcache-compile-job -Rcompile-job-cache &> %t/bad_key.txt
2747
// RUN: cat %t/bad_key.txt | FileCheck %s -check-prefix=BAD_KEY
2848

2949
// BAD_KEY: error: CAS cannot load module with key 'KEY' from -fmodule-file-cache-key: invalid cas-id 'KEY'
3050

31-
// RUN: echo -n '-fmodule-file-cache-key=PATH=' > %t/bad_key2.rsp
51+
// RUN: echo -n '-fmodule-file-cache-key PATH ' > %t/bad_key2.rsp
3252
// RUN: cat %t/casid >> %t/bad_key2.rsp
3353

3454
// RUN: not %clang_cc1 -triple x86_64-apple-macos11 \
@@ -56,7 +76,7 @@
5676

5777
// RUN: llvm-cas --cas %t.cas_2 --import --upstream-cas %t.cas @%t/A.key
5878

59-
// RUN: echo -n '-fmodule-file-cache-key=PATH=' > %t/not_in_cache.rsp
79+
// RUN: echo -n '-fmodule-file-cache-key PATH ' > %t/not_in_cache.rsp
6080
// RUN: cat %t/A.key >> %t/not_in_cache.rsp
6181

6282
// RUN: not %clang_cc1 -triple x86_64-apple-macos11 \

‎clang/test/CAS/fmodule-file-cache-key-lazy.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
// == Build A, importing B
2222

23-
// RUN: echo -n '-fmodule-file-cache-key=%t/B.pcm=' > %t/B.import.rsp
23+
// RUN: echo -n '-fmodule-file-cache-key %t/B.pcm ' > %t/B.import.rsp
2424
// RUN: cat %t/B.key >> %t/B.import.rsp
2525

2626
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \
@@ -34,7 +34,7 @@
3434

3535
// == Build tu, importing A (implicitly importing B)
3636

37-
// RUN: echo -n '-fmodule-file-cache-key=%t/A.pcm=' > %t/A.import.rsp
37+
// RUN: echo -n '-fmodule-file-cache-key %t/A.pcm ' > %t/A.import.rsp
3838
// RUN: cat %t/A.key >> %t/A.import.rsp
3939

4040
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \

‎clang/test/CAS/fmodule-file-cache-key-with-pch.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222

2323
// == Build A, importing B
2424

25-
// RUN: echo -n '-fmodule-file-cache-key=%t/B.pcm=' > %t/B.import.rsp
25+
// RUN: echo -n '-fmodule-file-cache-key %t/B.pcm ' > %t/B.import.rsp
2626
// RUN: cat %t/B.key >> %t/B.import.rsp
2727

2828
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \
@@ -49,7 +49,7 @@
4949

5050
// == Build PCH, importing A (implicitly importing B)
5151

52-
// RUN: echo -n '-fmodule-file-cache-key=%t/A.pcm=' > %t/A.import.rsp
52+
// RUN: echo -n '-fmodule-file-cache-key %t/A.pcm ' > %t/A.import.rsp
5353
// RUN: cat %t/A.key >> %t/A.import.rsp
5454

5555
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \
@@ -68,7 +68,7 @@
6868

6969
// == Build tu
7070

71-
// RUN: echo -n '-fmodule-file-cache-key=%t/C.pcm=' > %t/C.import.rsp
71+
// RUN: echo -n '-fmodule-file-cache-key %t/C.pcm ' > %t/C.import.rsp
7272
// RUN: cat %t/C.key >> %t/C.import.rsp
7373

7474
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \

‎clang/test/CAS/fmodule-file-cache-key.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
// == Build A, importing B
2222

23-
// RUN: echo -n '-fmodule-file-cache-key=%t/B.pcm=' > %t/B.import.rsp
23+
// RUN: echo -n '-fmodule-file-cache-key %t/B.pcm ' > %t/B.import.rsp
2424
// RUN: cat %t/B.key >> %t/B.import.rsp
2525

2626
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \
@@ -34,7 +34,7 @@
3434

3535
// == Build tu, importing A (implicitly importing B)
3636

37-
// RUN: echo -n '-fmodule-file-cache-key=%t/A.pcm=' > %t/A.import.rsp
37+
// RUN: echo -n '-fmodule-file-cache-key %t/A.pcm ' > %t/A.import.rsp
3838
// RUN: cat %t/A.key >> %t/A.import.rsp
3939

4040
// RUN: %clang_cc1 -triple x86_64-apple-macos11 \

‎clang/test/ClangScanDeps/modules-cas-fs-prefix-mapping.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@
6565
// CHECK: "-fmodule-map-file=/^src/module.modulemap"
6666
// CHECK: "-o"
6767
// CHECK: "[[PREFIX]]/modules/{{.*}}/A-{{.*}}.pcm"
68-
// CHECK: "-fmodule-file-cache-key=/^modules/{{.*}}/B-[[B_CONTEXT_HASH:[^.]+]].pcm=llvmcas://{{.*}}"
68+
// CHECK: "-fmodule-file-cache-key"
69+
// CHECK: "/^modules/{{.*}}/B-[[B_CONTEXT_HASH:[^.]+]].pcm"
70+
// CHECK: "llvmcas://{{.*}}"
6971
// CHECK: "-x"
7072
// CHECK: "c"
7173
// CHECK: "/^src/module.modulemap"
@@ -139,7 +141,9 @@
139141
// CHECK: "-fcas-fs-working-directory"
140142
// CHECK: "/^src"
141143
// CHECK: "-fmodule-map-file=/^src/module.modulemap"
142-
// CHECK: "-fmodule-file-cache-key=/^modules/{{.*}}A-{{.*}}.pcm=llvmcas://{{.*}}"
144+
// CHECK: "-fmodule-file-cache-key"
145+
// CHECK: "/^modules/{{.*}}A-{{.*}}.pcm"
146+
// CHECK: "llvmcas://{{.*}}"
143147
// CHECK: "-x"
144148
// CHECK: "c"
145149
// CHECK: "/^src/t.c"

‎clang/test/ClangScanDeps/modules-cas-trees-with-pch.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@
6767
// PCH-NEXT: "[[A_PCM:.*outputs.*A-.*\.pcm]]"
6868
// PCH: "-fcache-compile-job"
6969
// PCH: "-emit-module"
70-
// PCH: "-fmodule-file-cache-key=[[B_PCM:.*outputs.*B-.*\.pcm]]=[[B_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
70+
// PCH: "-fmodule-file-cache-key"
71+
// PCH: "[[B_PCM:.*outputs.*B-.*\.pcm]]"
72+
// PCH: "[[B_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
7173
// PCH: "-fmodule-file={{(B=)?}}[[B_PCM]]"
7274
// PCH: ]
7375
// PCH: "file-deps": [
@@ -116,7 +118,9 @@
116118
// PCH: "-fno-pch-timestamp"
117119
// PCH: "-fcache-compile-job"
118120
// PCH: "-emit-pch"
119-
// PCH: "-fmodule-file-cache-key=[[A_PCM]]={{llvmcas://[[:xdigit:]]+}}"
121+
// PCH: "-fmodule-file-cache-key"
122+
// PCH: "[[A_PCM]]"
123+
// PCH: "{{llvmcas://[[:xdigit:]]+}}"
120124
// PCH: "-fmodule-file={{(A=)?}}[[A_PCM]]"
121125
// PCH: ]
122126
// PCH: "file-deps": [
@@ -140,7 +144,9 @@
140144
// CHECK: "-fcache-compile-job"
141145
// CHECK: "-emit-module"
142146
// CHECK: "-fmodule-file={{(B=)?}}[[B_PCM:.*outputs.*B-.*\.pcm]]"
143-
// CHECK: "-fmodule-file-cache-key=[[B_PCM]]=[[B_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
147+
// CHECK: "-fmodule-file-cache-key"
148+
// CHECK: "[[B_PCM]]"
149+
// CHECK: "[[B_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
144150
// CHECK: ]
145151
// CHECK: "file-deps": [
146152
// CHECK-NEXT: "[[PREFIX]]{{.}}C.h"
@@ -167,7 +173,9 @@
167173
// CHECK-NEXT: "[[TU_ROOT_ID]]"
168174
// CHECK: "-fno-pch-timestamp"
169175
// CHECK: "-fcache-compile-job"
170-
// CHECK: "-fmodule-file-cache-key=[[C_PCM]]={{llvmcas://[[:xdigit:]]+}}"
176+
// CHECK: "-fmodule-file-cache-key"
177+
// CHECK: "[[C_PCM]]"
178+
// CHECK: "{{llvmcas://[[:xdigit:]]+}}"
171179
// CHECK: "-fmodule-file={{(C=)?}}[[C_PCM]]"
172180
// CHECK: ]
173181
// CHECK: "file-deps": [

‎clang/test/ClangScanDeps/modules-cas-trees.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@
8080
// CHECK-NEXT: "[[LEFT_PCM:.*outputs.*Left-.*\.pcm]]"
8181
// CHECK: "-fcache-compile-job"
8282
// CHECK: "-emit-module"
83-
// CHECK: "-fmodule-file-cache-key=[[TOP_PCM:.*outputs.*Top-.*\.pcm]]=[[TOP_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
83+
// CHECK: "-fmodule-file-cache-key"
84+
// CHECK: "[[TOP_PCM:.*outputs.*Top-.*\.pcm]]"
85+
// CHECK: "[[TOP_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
8486
// CHECK: "-fmodule-file={{(Top=)?}}[[TOP_PCM]]"
8587
// CHECK: ]
8688
// CHECK: "file-deps": [
@@ -106,7 +108,9 @@
106108
// CHECK-NEXT: "[[RIGHT_PCM:.*outputs.*Right-.*\.pcm]]"
107109
// CHECK: "-fcache-compile-job"
108110
// CHECK: "-emit-module"
109-
// CHECK: "-fmodule-file-cache-key=[[TOP_PCM]]=[[TOP_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
111+
// CHECK: "-fmodule-file-cache-key"
112+
// CHECK: "[[TOP_PCM]]"
113+
// CHECK: "[[TOP_CACHE_KEY:llvmcas://[[:xdigit:]]+]]"
110114
// CHECK: "-fmodule-file={{(Top=)?}}[[TOP_PCM]]"
111115
// CHECK: ]
112116
// CHECK: "file-deps": [
@@ -156,8 +160,12 @@
156160
// CHECK: "-fcas-fs"
157161
// CHECK-NEXT: "[[TU_ROOT_ID]]"
158162
// CHECK: "-fcache-compile-job"
159-
// CHECK: "-fmodule-file-cache-key=[[LEFT_PCM]]={{llvmcas://[[:xdigit:]]+}}"
160-
// CHECK: "-fmodule-file-cache-key=[[RIGHT_PCM]]={{llvmcas://[[:xdigit:]]+}}"
163+
// CHECK: "-fmodule-file-cache-key"
164+
// CHECK: "[[LEFT_PCM]]"
165+
// CHECK: "{{llvmcas://[[:xdigit:]]+}}"
166+
// CHECK: "-fmodule-file-cache-key"
167+
// CHECK: "[[RIGHT_PCM]]"
168+
// CHECK: "{{llvmcas://[[:xdigit:]]+}}"
161169
// CHECK: "-fmodule-file={{(Left=)?}}[[LEFT_PCM]]"
162170
// CHECK: "-fmodule-file={{(Right=)?}}[[RIGHT_PCM]]"
163171
// CHECK: ]

0 commit comments

Comments
 (0)