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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to