mboehme added inline comments.

================
Comment at: clang/lib/Sema/SemaType.cpp:170
     /// Whether we saved the attributes in the decl spec.
     bool hasSavedAttrs;
 
----------------
aaron.ballman wrote:
> Isn't the same true for this variable? It seems like:
> 
> `trivial` == `savedAttrs.empty()`
> `hasSavedAttrs` == `!savedAttrs.empty()`
That's what I also thought at first -- but the situation for `hasSavedAttrs` is 
a bit more complicated. It gets set whenever `saveDeclAttrs()` is called, even 
if it doesn't actually end up saving any attributes (i.e. if 
`spec.getAttributes()` is empty).

`hasSavedAttrs` is then used to prevent any further calls to 
`saveDeclSpecAttrs()` from doing anything:

```
// Don't try to save them multiple times.
if (hasSavedAttrs) return;
```

Conceivably, `spec` _might_ have had attributes added to it in the meantime -- 
not sure? It might be OK to just replace this logic with `if 
(!savedAttrs.empty()) return;` -- but I'm not familiar enough with how this 
code gets used and therefore decided it would be better not to change it. Can 
you give additional input on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123783

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

Reply via email to