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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
