[PATCH] D20959: [clang-format] make header guard identification stricter (with Lexer).

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 6 02:35:57 PDT 2016


djasper added inline comments.

================
Comment at: lib/Format/Format.cpp:1466
@@ +1465,3 @@
+            getFormattingLangOpts(Style));
+  unsigned Ret = Code.size();
+  Token Tok;
----------------
Don't store this. Just inline Code.size() below.

================
Comment at: lib/Format/Format.cpp:1471
@@ +1470,3 @@
+  skipComments(Lex, Tok);
+  Token TokAfterComments = Tok;
+  bool HeaderGuardFound = false;
----------------
I'd probably just store the offset, not the entire token.

================
Comment at: lib/Format/Format.cpp:1474
@@ +1473,3 @@
+  if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
+    skipComments(Lex, Tok);
+    if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
----------------
I wonder whether we should also skip comments after the header guard .. If so, we could probably just move the skipComments() call into checkAndConsumeDirectiveWithName.

================
Comment at: lib/Format/Format.cpp:1476
@@ +1475,3 @@
+    if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
+      HeaderGuardFound = true;
+  }
----------------
Maybe just return here and remove the HeaderGuardFound variable?

================
Comment at: lib/Format/Format.cpp:1480
@@ +1479,3 @@
+    Tok = TokAfterComments;
+  return Tok.isNot(tok::eof) ? SourceMgr.getFileOffset(Tok.getLocation())
+                             : Ret;
----------------
Does getFileOffset not work on the eof token?

================
Comment at: lib/Format/Format.cpp:1484
@@ +1483,3 @@
+
+// FIXME: we always +1 when we calculate the offset for each new line; however,
+// the last line does not necessarily end with '\n', which makes offset exceed
----------------
Seems like adding an std::min call would be shorter than this comment. Any reason not to just fix it now?


http://reviews.llvm.org/D20959





More information about the cfe-commits mailing list