[PATCH] D136710: [include-cleaner] Add the missing parts of Symbol and Header clases.
Kadir Cetinkaya via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 28 02:21:27 PDT 2022
kadircet added a comment.
thanks, i agree with your comments around `std::variant`, let's see what kind of patterns will emerge in the future.
so, still LG, but please address the triple slash issue and const-ness before landing
================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:40
+ enum Kind {
+ // A canonical clang declaration.
+ Declaration,
----------------
triple slashes (here and elsewhere)
================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h:60
+ // Order must match Kind enum!
+ std::variant<Decl *, tooling::stdlib::Symbol> Storage;
};
----------------
sammccall wrote:
> kadircet wrote:
> > nit: any particular reason for dropping const here?
> Oh, the constructor was nonconst so I assumed const was an oversight. Changed to const everywhere.
i guess you forgot to upload the diff? (the constructor was definitely an oversight, thanks for noticing that)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136710/new/
https://reviews.llvm.org/D136710
More information about the cfe-commits
mailing list