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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4062
+  if ((FLAGS)&options::CC1Option) {                                            \
+    const auto &Extracted = EXTRACTOR(this->KEYPATH);                          \
+    if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))                           \
----------------
IIUC, then we can do this more simply as:
```
bool Extracted = EXTRACTOR(this->KEYPATH);
```
That might clarify why we don't have lifetime extension concerns here.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3938
+  if ((FLAGS)&options::CC1Option) {                                            \
+    const auto &Extracted = EXTRACTOR(this->KEYPATH);                          \
+    if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE)                             \
----------------
jansvoboda11 wrote:
> Bigcheese wrote:
> > Will this ever have an issue with lifetime? I can see various values for `EXTRACTOR` causing issues here. https://abseil.io/tips/107
> > 
> > 
> > It would be good to at least document somewhere the restrictions on `EXTRACTOR`.
> Mentioned the reference lifetime extension in a comment near extractor definitions.
It might be safer to refactor as:
```
// Capture the extracted value as a lambda argument to avoid potential
// lifetime extension issues.
[&](const auto &Extracted) {
  if (ALWAYS_EMIT || Extracted != (DEFAULT_VALUE))
    DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, Extracted);
}(EXTRACTOR(this->KEYPATH));
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83211