This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG70410a264949: [clang][cli] Let denormalizer decide how to 
render the option based on theā€¦ (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84189/new/

https://reviews.llvm.org/D84189

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/unittests/Frontend/CompilerInvocationTest.cpp
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===================================================================
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -205,9 +205,6 @@
   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; }
 
Index: clang/unittests/Frontend/CompilerInvocationTest.cpp
===================================================================
--- clang/unittests/Frontend/CompilerInvocationTest.cpp
+++ clang/unittests/Frontend/CompilerInvocationTest.cpp
@@ -342,30 +342,70 @@
   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
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -152,8 +152,8 @@
 /// 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 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 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 @@
 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 @@
   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 @@
           (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));                                               \
   }
 
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4882,7 +4882,7 @@
   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">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D84189: [c... Jan Svoboda via Phabricator via cfe-commits
    • [PATCH] D8418... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D8418... Jan Svoboda via Phabricator via cfe-commits

Reply via email to