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; } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
