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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to