Skip to content

Commit 67a4c67

Browse files
jansvoboda11jansvoboda11
committed
Reapply "[clang][cli] Allow users to specify a conditional to prevent parsing options with MarshallingInfo"
This reverts commit d0fa7a0 and fixes failing OptionMarshallingTest by adding the SHOULD_PARSE macro argument
1 parent c9154e8 commit 67a4c67

File tree

6 files changed

+84
-30
lines changed

6 files changed

+84
-30
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4091,7 +4091,9 @@ defm sycl : BoolOption<"sycl",
40914091
BothFlags<[CoreOption], " SYCL kernels compilation for device">, "f">,
40924092
Group<sycl_Group>;
40934093
def sycl_std_EQ : Joined<["-"], "sycl-std=">, Group<sycl_Group>, Flags<[CC1Option, NoArgumentUnused, CoreOption]>,
4094-
HelpText<"SYCL language standard to compile for.">, Values<"2017, 121, 1.2.1, sycl-1.2.1">;
4094+
HelpText<"SYCL language standard to compile for.">, Values<"2017,121,1.2.1,sycl-1.2.1">,
4095+
NormalizedValues<["SYCL_2017", "SYCL_2017", "SYCL_2017", "SYCL_2017"]>, NormalizedValuesScope<"LangOptions">,
4096+
MarshallingInfoString<"LangOpts->SYCLVersion", "SYCL_None">, ShouldParseIf<fsycl.KeyPath>, AutoNormalizeEnum;
40954097

40964098
//===----------------------------------------------------------------------===//
40974099
// FlangOption and FC1 Options

‎clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2282,23 +2282,6 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
22822282
}
22832283

22842284
Opts.SYCLIsDevice = Opts.SYCL && Args.hasArg(options::OPT_fsycl_is_device);
2285-
if (Opts.SYCL) {
2286-
// -sycl-std applies to any SYCL source, not only those containing kernels,
2287-
// but also those using the SYCL API
2288-
if (const Arg *A = Args.getLastArg(OPT_sycl_std_EQ)) {
2289-
Opts.setSYCLVersion(
2290-
llvm::StringSwitch<LangOptions::SYCLMajorVersion>(A->getValue())
2291-
.Cases("2017", "1.2.1", "121", "sycl-1.2.1",
2292-
LangOptions::SYCL_2017)
2293-
.Default(LangOptions::SYCL_None));
2294-
2295-
if (Opts.getSYCLVersion() == LangOptions::SYCL_None) {
2296-
// User has passed an invalid value to the flag, this is an error
2297-
Diags.Report(diag::err_drv_invalid_value)
2298-
<< A->getAsString(Args) << A->getValue();
2299-
}
2300-
}
2301-
}
23022285

23032286
llvm::Triple T(TargetOpts.Triple);
23042287
CompilerInvocation::setLangDefaults(Opts, IK, T, PPOpts, LangStd);
@@ -3003,16 +2986,17 @@ bool CompilerInvocation::parseSimpleArgs(const ArgList &Args,
30032986
DiagnosticsEngine &Diags) {
30042987
#define OPTION_WITH_MARSHALLING( \
30052988
PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
3006-
HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, \
3007-
IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
3008-
TABLE_INDEX) \
2989+
HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, \
2990+
DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
2991+
MERGER, EXTRACTOR, TABLE_INDEX) \
30092992
if ((FLAGS)&options::CC1Option) { \
30102993
this->KEYPATH = MERGER(this->KEYPATH, DEFAULT_VALUE); \
30112994
if (IMPLIED_CHECK) \
30122995
this->KEYPATH = MERGER(this->KEYPATH, IMPLIED_VALUE); \
3013-
if (auto MaybeValue = NORMALIZER(OPT_##ID, TABLE_INDEX, Args, Diags)) \
3014-
this->KEYPATH = MERGER( \
3015-
this->KEYPATH, static_cast<decltype(this->KEYPATH)>(*MaybeValue)); \
2996+
if (SHOULD_PARSE) \
2997+
if (auto MaybeValue = NORMALIZER(OPT_##ID, TABLE_INDEX, Args, Diags)) \
2998+
this->KEYPATH = MERGER( \
2999+
this->KEYPATH, static_cast<decltype(this->KEYPATH)>(*MaybeValue)); \
30163000
}
30173001

30183002
#include "clang/Driver/Options.inc"
@@ -3265,9 +3249,9 @@ void CompilerInvocation::generateCC1CommandLine(
32653249
// with lifetime extension of the reference.
32663250
#define OPTION_WITH_MARSHALLING( \
32673251
PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
3268-
HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, \
3269-
IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
3270-
TABLE_INDEX) \
3252+
HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, \
3253+
DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
3254+
MERGER, EXTRACTOR, TABLE_INDEX) \
32713255
if ((FLAGS)&options::CC1Option) { \
32723256
[&](const auto &Extracted) { \
32733257
if (ALWAYS_EMIT || \

‎clang/unittests/Frontend/CompilerInvocationTest.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,68 @@ TEST_F(CommandLineTest, StringVectorMultiple) {
508508
ASSERT_THAT(GeneratedArgs, ContainsN(HasSubstr("-fmodule-map-file"), 2));
509509
}
510510

511+
// A flag that should be parsed only if a condition is met.
512+
513+
TEST_F(CommandLineTest, ConditionalParsingIfFalseFlagNotPresent) {
514+
const char *Args[] = {""};
515+
516+
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
517+
518+
ASSERT_FALSE(Diags->hasErrorOccurred());
519+
ASSERT_FALSE(Invocation.getLangOpts()->SYCL);
520+
ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_None);
521+
522+
Invocation.generateCC1CommandLine(GeneratedArgs, *this);
523+
524+
ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl"))));
525+
ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std="))));
526+
}
527+
528+
TEST_F(CommandLineTest, ConditionalParsingIfFalseFlagPresent) {
529+
const char *Args[] = {"-sycl-std=2017"};
530+
531+
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
532+
533+
ASSERT_FALSE(Diags->hasErrorOccurred());
534+
ASSERT_FALSE(Invocation.getLangOpts()->SYCL);
535+
ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_None);
536+
537+
Invocation.generateCC1CommandLine(GeneratedArgs, *this);
538+
539+
ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fsycl"))));
540+
ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std="))));
541+
}
542+
543+
TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagNotPresent) {
544+
const char *Args[] = {"-fsycl"};
545+
546+
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
547+
548+
ASSERT_FALSE(Diags->hasErrorOccurred());
549+
ASSERT_TRUE(Invocation.getLangOpts()->SYCL);
550+
ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_None);
551+
552+
Invocation.generateCC1CommandLine(GeneratedArgs, *this);
553+
554+
ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl")));
555+
ASSERT_THAT(GeneratedArgs, Not(Contains(HasSubstr("-sycl-std="))));
556+
}
557+
558+
TEST_F(CommandLineTest, ConditionalParsingIfTrueFlagPresent) {
559+
const char *Args[] = {"-fsycl", "-sycl-std=2017"};
560+
561+
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
562+
563+
ASSERT_FALSE(Diags->hasErrorOccurred());
564+
ASSERT_TRUE(Invocation.getLangOpts()->SYCL);
565+
ASSERT_EQ(Invocation.getLangOpts()->getSYCLVersion(), LangOptions::SYCL_2017);
566+
567+
Invocation.generateCC1CommandLine(GeneratedArgs, *this);
568+
569+
ASSERT_THAT(GeneratedArgs, Contains(StrEq("-fsycl")));
570+
ASSERT_THAT(GeneratedArgs, Contains(StrEq("-sycl-std=2017")));
571+
}
572+
511573
// Wide integer option.
512574

513575
TEST_F(CommandLineTest, WideIntegerHighValue) {

‎llvm/include/llvm/Option/OptParser.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ class Option<list<string> prefixes, string name, OptionKind kind> {
101101
code DefaultValue = ?;
102102
code ImpliedValue = ?;
103103
code ImpliedCheck = "false";
104+
code ShouldParse = "true";
104105
bit ShouldAlwaysEmit = false;
105106
code NormalizerRetTy = ?;
106107
code NormalizedValuesScope = "";
@@ -202,6 +203,7 @@ class MarshallingInfoBooleanFlag<code keypath, code defaultvalue, code value, co
202203

203204
// Mixins for additional marshalling attributes.
204205

206+
class ShouldParseIf<code condition> { code ShouldParse = condition; }
205207
class AlwaysEmit { bit ShouldAlwaysEmit = true; }
206208
class Normalizer<code normalizer> { code Normalizer = normalizer; }
207209
class Denormalizer<code denormalizer> { code Denormalizer = denormalizer; }

‎llvm/unittests/Option/OptionMarshallingTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ struct OptionWithMarshallingInfo {
1818
static const OptionWithMarshallingInfo MarshallingTable[] = {
1919
#define OPTION_WITH_MARSHALLING( \
2020
PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
21-
HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, \
22-
IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
23-
TABLE_INDEX) \
21+
HELPTEXT, METAVAR, VALUES, SPELLING, SHOULD_PARSE, ALWAYS_EMIT, KEYPATH, \
22+
DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, \
23+
MERGER, EXTRACTOR, TABLE_INDEX) \
2424
{NAME, #KEYPATH, #IMPLIED_CHECK, #IMPLIED_VALUE},
2525
#include "Opts.inc"
2626
#undef OPTION_WITH_MARSHALLING

‎llvm/utils/TableGen/OptParserEmitter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class MarshallingInfo {
7171
StringRef NormalizedValuesScope;
7272
StringRef ImpliedCheck;
7373
StringRef ImpliedValue;
74+
StringRef ShouldParse;
7475
StringRef Normalizer;
7576
StringRef Denormalizer;
7677
StringRef ValueMerger;
@@ -102,6 +103,8 @@ struct SimpleEnumValueTable {
102103
void emit(raw_ostream &OS) const {
103104
write_cstring(OS, StringRef(getOptionSpelling(R)));
104105
OS << ", ";
106+
OS << ShouldParse;
107+
OS << ", ";
105108
OS << ShouldAlwaysEmit;
106109
OS << ", ";
107110
OS << KeyPath;
@@ -167,6 +170,7 @@ static MarshallingInfo createMarshallingInfo(const Record &R) {
167170
Ret.ImpliedValue =
168171
R.getValueAsOptionalString("ImpliedValue").getValueOr(Ret.DefaultValue);
169172

173+
Ret.ShouldParse = R.getValueAsString("ShouldParse");
170174
Ret.Normalizer = R.getValueAsString("Normalizer");
171175
Ret.Denormalizer = R.getValueAsString("Denormalizer");
172176
Ret.ValueMerger = R.getValueAsString("ValueMerger");

0 commit comments

Comments
 (0)