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.
----------------
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?
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:93
+ // Require redeclarations only for definitions and only when the underlying
+ // type is specified.
----------------
This says what, rather than why.
Rather maybe:
```
Enums may be usefully forward-declared as *complete* types by specifying an
underlying type.
In this case, the definition should see the declaration so they can be checked
for compatibility.
```
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:96
+ bool VisitEnumDecl(EnumDecl *D) {
+ if (D != D->getDefinition() || !D->getIntegerTypeSourceInfo())
+ return false;
----------------
D->isThisDeclarationADefinition() is more idiomatic for the first part
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:98
+ return false;
+ for (const auto *Redecl : D->redecls())
+ Result.insert(Redecl->getLocation());
----------------
rather add(D) here?
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
