This is an archive of the discontinued LLVM Phabricator instance.

[clang-tblgen] renames Diagnostic.Text to Diagnostic.Summary
ClosedPublic

Authored by cjdb on Oct 12 2022, 3:16 PM.

Details

Summary

The [Improving Clang's Diagnostics RFC][1] identifies eight broad fields
for Clang to surface, two of which are text-based. Since the current
diagnostics more closely map to the diagnostic summary (or headline), we
should rename them to ensure that there's no confusion when
Diagnostic.Reason is introduced in the near future.

[1]: https://discourse.llvm.org/t/rfc-improving-clang-s-diagnostics/62584

Diff Detail

Event Timeline

cjdb created this revision.Oct 12 2022, 3:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2022, 3:16 PM
Herald added a subscriber: arphaman. · View Herald Transcript
cjdb requested review of this revision.Oct 12 2022, 3:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2022, 3:16 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
cjdb updated this revision to Diff 467283.Oct 12 2022, 3:42 PM

undoes an unnessecary change

erichkeane accepted this revision.Oct 13 2022, 6:16 AM

I'm unopposed. I think the justification isn't particularly clear to me and doesn't seem discussed in that RFC that I can tell, but I don't really have a concern here. Give folks a few days to complain if they are going to

This revision is now accepted and ready to land.Oct 13 2022, 6:16 AM
aaron.ballman accepted this revision.Oct 13 2022, 7:54 AM

LGTM as well!

Please be sure to add NFC to the commit message so it's clear why there's no tests.