Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

Differential D83691

Port Comment option flags to new parsing system
ClosedPublic
Actions

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

Details

Reviewers
Bigcheese
dang
dexonsmith
Commits
rGa1702a297b8b: [clang][cli] Port Comment option flags to new parsing system
Summary

Depends on D83690

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
Herald added subscribers: cfe-commits, dexonsmith. · View Herald Transcript
Harbormaster failed remote builds in B63989: Diff 277474!Jul 13 2020, 10:12 AM
dang added a child revision: D83693: Port analyzer flags to new option parsing system.Jul 13 2020, 10:12 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.

jansvoboda11 updated this revision to Diff 306042.Nov 18 2020, 4:05 AM

Rebase

jansvoboda11 added a comment.Nov 18 2020, 4:05 AM

WDYT @Bigcheese?

Harbormaster completed remote builds in B79256: Diff 306042.Nov 18 2020, 4:06 AM
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
jansvoboda11 added a comment.Nov 19 2020, 6:23 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
Closed by commit rGa1702a297b8b: [clang][cli] Port Comment option flags to new parsing system (authored by jansvoboda11). · Explain Why
This revision was automatically updated to reflect the committed changes.
jansvoboda11 added a commit: rGa1702a297b8b: [clang][cli] Port Comment option flags to new parsing system.

Revision Contents
Changeset List

PathSize
clang/
include/
clang/
Driver/
3 lines

Diff 306397

View Options

clang/include/clang/Driver/Options.td

Loading...