[Diagnostics][NFCI] Introduce Structured fix-its #26612
Conversation
owenv
force-pushed
the
formatted_fixits
branch
from
13e6de8 to
dd3d2cf
Compare
|
Rebased and moved some of the examples over to CSDiagnostics now that those diags have been ported over 🎉 |
|
Hi @xedin / @jrose-apple , just wanted to bump this. Totally understand if this change is undesirable, but I have had some success leveraging it to fix type shadowing issues in fix-its by optionally attaching a DeclContext which can be used to determine how a fix-it’s arguments should be formatted (that would be a follow-up PR). |
There was a problem hiding this comment.
This seems reasonable to me, but I'd like @jrose-apple to take a look as well.
owenv
force-pushed
the
formatted_fixits
branch
2 times, most recently
from
4ca0016 to
e69e73c
Compare
|
Hi @jrose-apple , I'd like to pick this back up if possible. Do you mind taking a look when you have time? |
lib/AST/DiagnosticList.cpp
Outdated
| @@ -24,6 +24,11 @@ enum class swift::DiagID : uint32_t { | |||
| static_assert(static_cast<uint32_t>(swift::DiagID::invalid_diagnostic) == 0, | |||
| "0 is not the invalid diagnostic ID"); | |||
|
|
|||
| enum class swift::FixItID : uint32_t { | |||
| #define FIXIT(ID, Text, Signature) ID, | |||
| #include "swift/AST/FixIts.def" | |||
There was a problem hiding this comment.
Does it make sense to have fix-its in their own file? I would have thought it'd be simplest to have them next to the diagnostics.
There was a problem hiding this comment.
Defining fix-its alongside the other diags complicates the macro usage by a fair amount. I'm starting to think the advantages are outweigh the tradeoffs though, so updated the PR to do this. I only added the definitions to Parse and Sema diags for now, they can be rolled out to the other headers as needed.
owenv
force-pushed
the
formatted_fixits
branch
from
e69e73c to
bc11abd
Compare
owenv
force-pushed
the
formatted_fixits
branch
2 times, most recently
from
624f236 to
1c30c54
Compare
owenv
force-pushed
the
formatted_fixits
branch
2 times, most recently
from
5687a76 to
873e04d
Compare
|
Thanks for bearing with me as I fix up these remaining issues! |
These are defined with macros like errors/warnings/notes, and make use of format strings and diagnostic arguments. The intent is to leverage diagnostic arguments in the future to disambiguate ambiguously spelled types. Ported a few miscellaneous fix-its to the new system
owenv
force-pushed
the
formatted_fixits
branch
from
873e04d to
68e6065
Compare
|
Welcome to C++. :-) @swift-ci Please smoke test |
These are defined with macros like errors/warnings/notes, and make use of format strings and diagnostic arguments. The intent is to leverage diagnostic arguments in the future to disambiguate ambiguously spelled types.
Ported a few miscellaneous fix-its to the new system.
Some of the rationale for this change is explained here. This change alone isn't enough to fix e.g. SR-8668, but it's a first step towards being able to handle that kind of situation better.
If this seems like a good direction to take, the next steps I had in mind are roughly:
cc @jrose-apple (this builds on some of the argument formatting changes to diags from a few weeks back)
cc @xedin (this may eventually affect many fix-its produced during type checking)