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