[PATCH] D83211: Factor out call to EXTRACTOR in generateCC1CommandLine

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 307837.
jansvoboda11 added a comment.
Herald added subscribers: llvm-commits, mgorny.
Herald added a project: LLVM.

Implement review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  llvm/unittests/Option/CMakeLists.txt
  llvm/unittests/Option/LifetimeOpts.td


Index: llvm/unittests/Option/LifetimeOpts.td
===================================================================
--- /dev/null
+++ llvm/unittests/Option/LifetimeOpts.td
@@ -0,0 +1,5 @@
+include "llvm/Option/OptParser.td"
+
+def lifetime_test : Flag<["-"], "lifetime-test">,
+  MarshallingInfoFlag<"FlagValue">,
+  ValueExtractor<"extractorRequiringLifetimeExtension">;
Index: llvm/unittests/Option/CMakeLists.txt
===================================================================
--- llvm/unittests/Option/CMakeLists.txt
+++ llvm/unittests/Option/CMakeLists.txt
@@ -4,8 +4,11 @@
   )
 
 set(LLVM_TARGET_DEFINITIONS Opts.td)
-
 tablegen(LLVM Opts.inc -gen-opt-parser-defs)
+
+set(LLVM_TARGET_DEFINITIONS LifetimeOpts.td)
+tablegen(LLVM LifetimeOpts.inc -gen-opt-parser-defs)
+
 add_public_tablegen_target(OptsTestTableGen)
 
 add_llvm_unittest(OptionTests
Index: clang/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -263,6 +263,9 @@
   return KeyPath | Value;
 }
 
+// The value returned by an extractor is stored as a constant reference.
+// Watch out for missed reference lifetime extension.
+
 template <typename T> static T extractForwardValue(T KeyPath) {
   return KeyPath;
 }
@@ -4015,13 +4018,17 @@
 
 void CompilerInvocation::generateCC1CommandLine(
     SmallVectorImpl<const char *> &Args, StringAllocator SA) const {
+  // Capture the extracted value as a lambda argument to avoid potential issues
+  // with lifetime extension of the reference.
 #define OPTION_WITH_MARSHALLING(                                               \
     PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,        \
     HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
     NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX)                  \
-  if (((FLAGS) & options::CC1Option) &&                                        \
-      (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != (DEFAULT_VALUE))) {          \
-    DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   \
+  if ((FLAGS)&options::CC1Option) {                                            \
+    [&](const auto &Extracted) {                                               \
+      if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))                         \
+        DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);              \
+    }(EXTRACTOR(this->KEYPATH));                                               \
   }
 
 #define OPTION_WITH_MARSHALLING_BOOLEAN(                                       \
@@ -4029,10 +4036,10 @@
     HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE,  \
     NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, TABLE_INDEX, NEG_ID,          \
     NEG_SPELLING)                                                              \
-  if (((FLAGS)&options::CC1Option) &&                                          \
-      (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {            \
-    DENORMALIZER(Args, SPELLING, NEG_SPELLING, SA, TABLE_INDEX,                \
-                 EXTRACTOR(this->KEYPATH));                                    \
+  if ((FLAGS)&options::CC1Option) {                                            \
+    bool Extracted = EXTRACTOR(this->KEYPATH);                                 \
+    if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))                           \
+      DENORMALIZER(Args, SPELLING, NEG_SPELLING, SA, TABLE_INDEX, Extracted);  \
   }
 
 #include "clang/Driver/Options.inc"


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83211.307837.patch
Type: text/x-patch
Size: 3653 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201126/3b166fb7/attachment.bin>