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

LGTM after you fix a couple of nits.



================
Comment at: clang/include/clang/Driver/Options.td:405
 
+// Comment Options
+
----------------
Nit: please end the comment with a period to make this a sentence; I'm not sure 
if "Options" is really a proper noun; I would have thought "Comment options.".


================
Comment at: clang/include/clang/Driver/Options.td:407
+
+def fparse_all_comments : Flag<["-"], "fparse-all-comments">, 
Group<f_clang_Group>, Flags<[CC1Option]>,
+  MarshallingInfoFlag<"LangOpts->CommentOpts.ParseAllComments">;
----------------
Nit: is moving this option and adding a comment a necessary part of the patch, 
or is it just a cleanup that could be done before in an NFC commit? If it's 
possible, I would suggest moving it in a separate commit.


================
Comment at: clang/include/clang/Driver/Options.td:410
+
 // Standard Options
 
----------------
Might be nice to clean up this comment in a separate NFC commit ahead of time, 
to match what you do with "Comment Options".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83691

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

Reply via email to