[PATCH] D131396: [pseudo] Use C++17 variant to simplify the DirectiveTree::Chunk class, NFC.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 9 22:35:15 PDT 2022


sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice!



================
Comment at: clang-tools-extra/pseudo/lib/DirectiveTree.cpp:187
+    Dumper dumper(OS);                                                         \
+    dumper(T);                                                                 \
     return OS;                                                                 \
----------------
Or Dumper{OS}(T)


================
Comment at: clang-tools-extra/pseudo/lib/DirectiveTree.cpp:214
 
   void choose(DirectiveTree &M) { walk(M); }
 
----------------
walk/choose seem redundant now, inline?


================
Comment at: clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp:47
 
-MATCHER_P(chunkKind, K, "") { return arg.kind() == K; }
+MATCHER(chunkDirective, "") {
+  return std::holds_alternative<DirectiveTree::Directive>(arg);
----------------
nit: directiveChunk() etc would be more natural, matching adjective order in English


================
Comment at: clang-tools-extra/pseudo/unittests/DirectiveTreeTest.cpp:124
+              tokensAre(S, "/*A*/"));
+  const DirectiveTree::Directive &Define =
+      std::get<DirectiveTree::Directive>(PP.Chunks[1]);
----------------
Nit: I'd always use `auto&` with std::get as the type is obvious


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131396/new/

https://reviews.llvm.org/D131396



More information about the cfe-commits mailing list