[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