[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