[PATCH] D83691: Port Comment option flags to new parsing system

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 18 11:26:11 PST 2020


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



More information about the cfe-commits mailing list