This is an archive of the discontinued LLVM Phabricator instance.

Port Comment option flags to new parsing system
ClosedPublic

Authored by jansvoboda11 on Jul 13 2020, 10:11 AM.

Diff Detail

Event Timeline

dang created this revision.Jul 13 2020, 10:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 13 2020, 10:11 AM
jansvoboda11 commandeered this revision.Nov 18 2020, 4:04 AM
jansvoboda11 added a reviewer: dang.
jansvoboda11 added a subscriber: jansvoboda11.

Taking over this patch, as Daniel is no longer involved.

dexonsmith accepted this revision.Nov 18 2020, 11:26 AM

LGTM after you fix a couple of nits.

clang/include/clang/Driver/Options.td
405

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.".

405

Might be nice to clean up this comment in a separate NFC commit ahead of time, to match what you do with "Comment Options".

407

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.

This revision is now accepted and ready to land.Nov 18 2020, 11:26 AM

Thanks. Committing this without the NFC change.

clang/include/clang/Driver/Options.td
407

That's a fair point, moving the option is not necessary. I will revert the move in this patch and consider creating the // Comment options. section once all comment options get ported to the marshaling system.

This revision was landed with ongoing or failed builds.Nov 19 2020, 6:31 AM
This revision was automatically updated to reflect the committed changes.