-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang] Enable descriptions for --print-supported-extensions #66715
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really nice. Two overall things:
Now that ARM and AArch64 print more than just names, they should have column headers like RISC-V does. Name / Description
The clang test case clang/test/Driver/print-supported-extensions.c
should be extended to check at least the column names and 1 row of the extension details. This is because we're now combining a few functions to produce this output, so an integration test is needed.
It is a bit ugly to have to combine these 2 sources of information but that's a larger issue that shouldn't block this.
Thank you for the review time, will address all the concerns and be back with a re-review request. |
The new output: console-output.txt.gz
Addressed all requests, let me know if still spot something. |
Addressed all requests.
|
Looks good to me, thanks for working on this. I'm not sure if you'll have permissions to click the merge button but if you do, go ahead. Please remember to edit the final commit message to remove the links to the output files. |
Thank you very much for the time !
|
If you are doing more contributions you can get commit access by doing https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access. Though with the switch to Github it's super easy to ask the reviewer to click the merge button, so it's not required. |
Enables summary descriptions along with the names of the feature.
Descriptions here are simply looked up via the available llvm tablegen data.