sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:38
bool VisitTagType(TagType *TT) {
+ // For enumerations we will require only the definition if it's present and
+ // the underlying type is not specified.
----------------
kbobyrev wrote:
> sammccall wrote:
> > I don't understand this special case.
> > It seems you're trying to avoid requiring too many redecls of a referenced
> > type. Why is this important, and different from e.g. Class?
> Yes, this is for cases like
>
> ```
> {
> "enum class Color;",
> "enum class Color {}; Color c;",
> },
> ```
>
> I think the problem here is that if we see the definition of the enum, it
> should be the "ground truth" for the usage, forward declarations are not
> really useful in this case. It's not much different from the class but I just
> wanted to handle it separately, not sure if it's OK to merge these two
> changes into one patch.
The heuristic seems plausible, though there are counterexamples like:
```
"enum Color;"
"void foo(Color); enum Color {};"
```
My main argument would be that we have a general policy here: conservatively
consider all redecls as used, rather than trying to work out which is best.
It's not clear why enums are special enough that we should hack around them
without rethinking the policy.
(I think `VisitEnumDecl` is different - our general policy is that decls
*aren't* references to forward-decls, and in enums' case this is violates our
meta-policy of "be conservative, consider everything that might be a use").
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112209/new/
https://reviews.llvm.org/D112209
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
