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

LGTM after you add the testcase I mention.



================
Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:623
+    // #pragma once
+    skipLine(First, End);
+    makeToken(pp_pragma_once);
----------------
I wondered if it was wrong to ignore extra token on the line, but `clang` does 
as well (and emits `-Wextra-tokens`) so this is fine.


================
Comment at: 
clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp:551-555
+  StringRef Source = R"(// comment
+#pragma once
+// another comment
+#include <test.h>
+)";
----------------
Can you also add a test with `#pragma once extra tokens`?  It should get 
canonicalized to `#pragma once`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64945



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D64945: [c... Alex Lorenz via Phabricator via cfe-commits
    • [PATCH] D6494... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D6494... Alex Lorenz via Phabricator via cfe-commits

Reply via email to