[llvm-branch-commits] [clang] 70410a2 - [clang][cli] Let denormalizer decide how to render the option based on the option class
Jan Svoboda via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Dec 21 02:37:27 PST 2020
Author: Jan Svoboda
Date: 2020-12-21T11:32:47+01:00
New Revision: 70410a264949101ced3ce3458f37dd4cc2f5af85
URL: https://github.com/llvm/llvm-project/commit/70410a264949101ced3ce3458f37dd4cc2f5af85
DIFF: https://github.com/llvm/llvm-project/commit/70410a264949101ced3ce3458f37dd4cc2f5af85.diff
LOG: [clang][cli] Let denormalizer decide how to render the option based on the option class
Before this patch, you needed to use `AutoNormalizeEnumJoined` whenever you wanted to **de**normalize joined enum.
Besides the naming confusion, this means the fact the option is joined is specified in two places: in the normalization multiclass and in the `Joined<["-"], ...>` multiclass.
This patch makes this work automatically, taking into account the `OptionClass` of options.
Also, the enum denormalizer now just looks up the spelling of the present enum case in a table and forwards it to the string denormalizer.
I also added more tests that exercise this.
Reviewed By: dexonsmith
Original patch by Daniel Grumberg.
Differential Revision: https://reviews.llvm.org/D84189
Added:
Modified:
clang/include/clang/Driver/Options.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/unittests/Frontend/CompilerInvocationTest.cpp
llvm/include/llvm/Option/OptParser.td
Removed:
################################################################################
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 29ee948f1849..82c4e9399d9d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4882,7 +4882,7 @@ def arcmt_action_EQ : Joined<["-"], "arcmt-action=">, Flags<[CC1Option, NoDriver
NormalizedValuesScope<"FrontendOptions">,
NormalizedValues<["ARCMT_Check", "ARCMT_Modify", "ARCMT_Migrate"]>,
MarshallingInfoString<"FrontendOpts.ARCMTAction", "ARCMT_None">,
- AutoNormalizeEnumJoined;
+ AutoNormalizeEnum;
def opt_record_file : Separate<["-"], "opt-record-file">,
HelpText<"File name to use for YAML optimization record output">,
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 00615a70d730..f71b14eabc49 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -152,8 +152,8 @@ static Optional<bool> normalizeSimpleNegativeFlag(OptSpecifier Opt, unsigned,
/// argument.
static void denormalizeSimpleFlag(SmallVectorImpl<const char *> &Args,
const char *Spelling,
- CompilerInvocation::StringAllocator, unsigned,
- /*T*/...) {
+ CompilerInvocation::StringAllocator,
+ Option::OptionClass, unsigned, /*T*/...) {
Args.push_back(Spelling);
}
@@ -200,12 +200,41 @@ static auto makeBooleanOptionNormalizer(bool Value, bool OtherValue,
static auto makeBooleanOptionDenormalizer(bool Value) {
return [Value](SmallVectorImpl<const char *> &Args, const char *Spelling,
- CompilerInvocation::StringAllocator, unsigned, bool KeyPath) {
+ CompilerInvocation::StringAllocator, Option::OptionClass,
+ unsigned, bool KeyPath) {
if (KeyPath == Value)
Args.push_back(Spelling);
};
}
+static void denormalizeStringImpl(SmallVectorImpl<const char *> &Args,
+ const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ Option::OptionClass OptClass, unsigned,
+ Twine Value) {
+ switch (OptClass) {
+ case Option::SeparateClass:
+ case Option::JoinedOrSeparateClass:
+ Args.push_back(Spelling);
+ Args.push_back(SA(Value));
+ break;
+ case Option::JoinedClass:
+ Args.push_back(SA(Twine(Spelling) + Value));
+ break;
+ default:
+ llvm_unreachable("Cannot denormalize an option with option class "
+ "incompatible with string denormalization.");
+ }
+}
+
+template <typename T>
+static void
+denormalizeString(SmallVectorImpl<const char *> &Args, const char *Spelling,
+ CompilerInvocation::StringAllocator SA,
+ Option::OptionClass OptClass, unsigned TableIndex, T Value) {
+ denormalizeStringImpl(Args, Spelling, SA, OptClass, TableIndex, Twine(Value));
+}
+
static Optional<SimpleEnumValue>
findValueTableByName(const SimpleEnumValueTable &Table, StringRef Name) {
for (int I = 0, E = Table.Size; I != E; ++I)
@@ -247,12 +276,13 @@ static llvm::Optional<unsigned> normalizeSimpleEnum(OptSpecifier Opt,
static void denormalizeSimpleEnumImpl(SmallVectorImpl<const char *> &Args,
const char *Spelling,
CompilerInvocation::StringAllocator SA,
+ Option::OptionClass OptClass,
unsigned TableIndex, unsigned Value) {
assert(TableIndex < SimpleEnumValueTablesSize);
const SimpleEnumValueTable &Table = SimpleEnumValueTables[TableIndex];
if (auto MaybeEnumVal = findValueTableByValue(Table, Value)) {
- Args.push_back(Spelling);
- Args.push_back(MaybeEnumVal->Name);
+ denormalizeString(Args, Spelling, SA, OptClass, TableIndex,
+ MaybeEnumVal->Name);
} else {
llvm_unreachable("The simple enum value was not correctly defined in "
"the tablegen option description");
@@ -263,24 +293,12 @@ template <typename T>
static void denormalizeSimpleEnum(SmallVectorImpl<const char *> &Args,
const char *Spelling,
CompilerInvocation::StringAllocator SA,
+ Option::OptionClass OptClass,
unsigned TableIndex, T Value) {
- return denormalizeSimpleEnumImpl(Args, Spelling, SA, TableIndex,
+ return denormalizeSimpleEnumImpl(Args, Spelling, SA, OptClass, TableIndex,
static_cast<unsigned>(Value));
}
-static void denormalizeSimpleEnumJoined(SmallVectorImpl<const char *> &Args,
- const char *Spelling,
- CompilerInvocation::StringAllocator SA,
- unsigned TableIndex, unsigned Value) {
- assert(TableIndex < SimpleEnumValueTablesSize);
- const SimpleEnumValueTable &Table = SimpleEnumValueTables[TableIndex];
- if (auto MaybeEnumVal = findValueTableByValue(Table, Value))
- Args.push_back(SA(Twine(Spelling) + MaybeEnumVal->Name));
- else
- llvm_unreachable("The simple enum value was not correctly defined in "
- "the tablegen option description");
-}
-
static Optional<std::string> normalizeString(OptSpecifier Opt, int TableIndex,
const ArgList &Args,
DiagnosticsEngine &Diags) {
@@ -290,25 +308,6 @@ static Optional<std::string> normalizeString(OptSpecifier Opt, int TableIndex,
return std::string(Arg->getValue());
}
-static void denormalizeString(SmallVectorImpl<const char *> &Args,
- const char *Spelling,
- CompilerInvocation::StringAllocator SA, unsigned,
- Twine Value) {
- Args.push_back(Spelling);
- Args.push_back(SA(Value));
-}
-
-template <typename T,
- std::enable_if_t<!std::is_convertible<T, Twine>::value &&
- std::is_constructible<Twine, T>::value,
- bool> = false>
-static void denormalizeString(SmallVectorImpl<const char *> &Args,
- const char *Spelling,
- CompilerInvocation::StringAllocator SA,
- unsigned TableIndex, T Value) {
- denormalizeString(Args, Spelling, SA, TableIndex, Twine(Value));
-}
-
template <typename IntTy>
static Optional<IntTy> normalizeStringIntegral(OptSpecifier Opt, int,
const ArgList &Args,
@@ -3267,7 +3266,8 @@ void CompilerInvocation::generateCC1CommandLine(
(Extracted != \
static_cast<decltype(this->KEYPATH)>( \
(IMPLIED_CHECK) ? (IMPLIED_VALUE) : (DEFAULT_VALUE)))) \
- DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted); \
+ DENORMALIZER(Args, SPELLING, SA, Option::KIND##Class, TABLE_INDEX, \
+ Extracted); \
}(EXTRACTOR(this->KEYPATH)); \
}
diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp
index 51b7ba8c147f..71e8d0907fc8 100644
--- a/clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -342,30 +342,70 @@ TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateRequiredAbsent) {
ASSERT_THAT(GeneratedArgs, Contains(StrEq(DefaultTriple.c_str())));
}
-TEST_F(CommandLineTest, CanGenerateCC1CommandLineSeparateEnumNonDefault) {
+TEST_F(CommandLineTest, SeparateEnumNonDefault) {
const char *Args[] = {"-mrelocation-model", "static"};
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
ASSERT_FALSE(Diags->hasErrorOccurred());
+ ASSERT_EQ(Invocation.getCodeGenOpts().RelocationModel, Reloc::Model::Static);
Invocation.generateCC1CommandLine(GeneratedArgs, *this);
// Non default relocation model.
+ ASSERT_THAT(GeneratedArgs, Contains(StrEq("-mrelocation-model")));
ASSERT_THAT(GeneratedArgs, Contains(StrEq("static")));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-mrelocation-model=static"))));
}
-TEST_F(CommandLineTest, CanGenerateCC1COmmandLineSeparateEnumDefault) {
+TEST_F(CommandLineTest, SeparateEnumDefault) {
const char *Args[] = {"-mrelocation-model", "pic"};
CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
ASSERT_FALSE(Diags->hasErrorOccurred());
+ ASSERT_EQ(Invocation.getCodeGenOpts().RelocationModel, Reloc::Model::PIC_);
Invocation.generateCC1CommandLine(GeneratedArgs, *this);
// Default relocation model.
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-mrelocation-model"))));
ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("pic"))));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-mrelocation-model=pic"))));
+}
+
+TEST_F(CommandLineTest, JoinedEnumNonDefault) {
+ const char *Args[] = {"-fobjc-dispatch-method=non-legacy"};
+
+ CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+ ASSERT_FALSE(Diags->hasErrorOccurred());
+ ASSERT_EQ(Invocation.getCodeGenOpts().getObjCDispatchMethod(),
+ CodeGenOptions::NonLegacy);
+
+ Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+ ASSERT_THAT(GeneratedArgs,
+ Contains(StrEq("-fobjc-dispatch-method=non-legacy")));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fobjc-dispatch-method="))));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("non-legacy"))));
+}
+
+TEST_F(CommandLineTest, JoinedEnumDefault) {
+ const char *Args[] = {"-fobjc-dispatch-method=legacy"};
+
+ CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags);
+
+ ASSERT_FALSE(Diags->hasErrorOccurred());
+ ASSERT_EQ(Invocation.getCodeGenOpts().getObjCDispatchMethod(),
+ CodeGenOptions::Legacy);
+
+ Invocation.generateCC1CommandLine(GeneratedArgs, *this);
+
+ ASSERT_THAT(GeneratedArgs,
+ Not(Contains(StrEq("-fobjc-dispatch-method=legacy"))));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("-fobjc-dispatch-method="))));
+ ASSERT_THAT(GeneratedArgs, Not(Contains(StrEq("legacy"))));
}
// Tree of boolean options that can be (directly or transitively) implied by
diff --git a/llvm/include/llvm/Option/OptParser.td b/llvm/include/llvm/Option/OptParser.td
index 98acce2ae4aa..d7d4e03b15f0 100644
--- a/llvm/include/llvm/Option/OptParser.td
+++ b/llvm/include/llvm/Option/OptParser.td
@@ -205,9 +205,6 @@ class AutoNormalizeEnum {
code Normalizer = "normalizeSimpleEnum";
code Denormalizer = "denormalizeSimpleEnum";
}
-class AutoNormalizeEnumJoined : AutoNormalizeEnum {
- code Denormalizer = "denormalizeSimpleEnumJoined";
-}
class ValueMerger<code merger> { code ValueMerger = merger; }
class ValueExtractor<code extractor> { code ValueExtractor = extractor; }
More information about the llvm-branch-commits
mailing list