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

Eric Liu via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 6 03:19:09 PDT 2016


ioeric added inline comments.

================
Comment at: lib/Format/Format.cpp:1474
@@ +1473,3 @@
+  if (checkAndConsumeDirectiveWithName(Lex, "ifndef", Tok)) {
+    skipComments(Lex, Tok);
+    if (checkAndConsumeDirectiveWithName(Lex, "define", Tok))
----------------
djasper wrote:
> I wonder whether we should also skip comments after the header guard .. If so, we could probably just move the skipComments() call into checkAndConsumeDirectiveWithName.
Not sure if we want to skip those too...seems to me that comments following header guard do not usually belong to header guard...they would most often belong to the code after it I guess? 

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

================
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
----------------
djasper wrote:
> Seems like adding an std::min call would be shorter than this comment. Any reason not to just fix it now?
We actually need more than that. For example, adding '\n' after the last line etc. This is not quite trivial to me since '\n' could be inserted after header insertions with the same offset.

I thought it would be better to do the fix in a separate patch. But I'll fix the crash here and handle '\n' insertion in the future.


http://reviews.llvm.org/D20959





More information about the cfe-commits mailing list