[PATCH] D83071: Add support for options with two flags for controlling the same field.

[Daniel Grumberg via Phabricator via cfe-commits]

dang marked an inline comment as done.
dang added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3931-3932
   if (((FLAGS)&options::CC1Option) &&                                          \
       (ALWAYS_EMIT || EXTRACTOR(this->KEYPATH) != DEFAULT_VALUE)) {            \
-    if (Option::KIND##Class == Option::FlagClass) {                            \
-      Args.push_back(SPELLING);                                                \
-    }                                                                          \
-    if (Option::KIND##Class == Option::SeparateClass) {                        \
-      Args.push_back(SPELLING);                                                \
-      DENORMALIZER(Args, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));           \
-    }                                                                          \
+    DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));   \
   }
----------------
dang wrote:
> dexonsmith wrote:
> > I realize this commit doesn't introduce it, but it seems unfortunate to call `EXTRACTOR` twice. Maybe in a follow-up or prep commit you can fix that... maybe something like this?
> > ```
> >   if ((FLAGS)&options::CC1Option) {
> >     const auto &Extracted = EXTRACTOR(this->KEYPATH);
> >     if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE)
> >       DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH));
> >   }
> > ```
> Yes I can do that of course. Although EXTRACTOR is meant to be very cheap and in most cases it expands to just `this->KEYPATH`
See D83211


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83071