Page MenuHomePhabricator

[TableGen] [Clang] Clean up Options.td and add asserts
ClosedPublic

Authored by Paul-C-Anagnostopoulos on May 3 2021, 10:14 AM.

Details

Summary

This revision does some cleanup to Options.td. It also adds three asserts that have been listed as TODOs for awhile.

Diff Detail

Event Timeline

Paul-C-Anagnostopoulos requested review of this revision.May 3 2021, 10:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2021, 10:14 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
dblaikie accepted this revision.May 3 2021, 4:10 PM

Sounds good

This revision is now accepted and ready to land.May 3 2021, 4:10 PM
jansvoboda11 accepted this revision.May 4 2021, 12:47 AM

Nice, thank you, Paul!

clang/include/clang/Driver/Options.td
1095

Were you planning to refactor this too?

clang/include/clang/Driver/Options.td
1095

You can't use the paste operator (#) there because c2x is a defvar. Defvars and undefined identifiers are taken literally by paste, due to its frequent use in forming record and class names.

This behavior is noted in the Programmer's Reference, but I think I will add a more prominent note. I wonder if we should change the behavior so defvars are treated normally.

jansvoboda11 added inline comments.May 4 2021, 4:51 AM
clang/include/clang/Driver/Options.td
1095

I see. In that case, can you please undo the whitespace change?

clang/include/clang/Driver/Options.td
1095

Sure, but don't we try to avoid over-long lines in .td files, too?

Restored whitespace as requested by Jan.

Thanks Paul!

clang/include/clang/Driver/Options.td
1095

I think that would be nice, but I'm not sure what's LLVM's policy on making whitespace-only changes (it makes git blame less useful). If we decide that's fine, let's do it in a purely NFC commit.

dblaikie added inline comments.May 7 2021, 1:35 PM
clang/include/clang/Driver/Options.td
1095

Yeah - generally the attitude is to not wholesale reformat files generally, but if you're going to make a bunch of changes in a file that's out of conformance it can be reasonable/worthwhile to reformat that file before the work - so that the autoformatted changes aren't in conflict with the existing format of the file. (& yeah, format changes outside the semantic lines being changed in a patch is undesirable & should be done separately if it's being done at all)