ghost
commented
12 months ago Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas See info in area-owners.md if you want to be subscribed.
Issue Details
Analyzers are compiled against a specific version of the C# compiler. As such they can only recognize the shapes of the code that version of the compiler supports. With a new version of the compiler new shapes become possible, sometimes these are subtle changes, sometimes they mean adding new node types to representation or new enum values to properties. Since we can't update analyzers in sync with the compiler, the analyzer has to be written such that it can survive most of these changes without too much harm (typically it would simply ignore the new shape). But that means that we don't get to be notified if such a new shape start occurring and can't fix the analyzer unless we have some other reason to look into that case. There will be cases where the analyzer doesn't work the way it should simply because it wasn't updated to match the latest compiler behavior. We considered adding a warning into the analyzer to ask the user to file an issue if this happens, but that comes with rather severe downsides: * If this happens, it's not an actionable warning (other than filing the bug), the only solution is to suppress the warning code * Lot of users treat warnings as errors, so this becomes a blocking problem for them if it happens * We're putting burden on our users due to our inability to keep things in sync (See the discussion in https://github.com/dotnet/runtime/pull/88836 for some more detail) For the illink analyzer we decided to change these cases into assertions. But that means we would like to run the debug build of the analyzer on as much code as possible so that we catch these things internally when they happen. I think it would make sense to do this for all the analyzers where it's possible - there are assertions in their code bases as well which are only exercised on targeted unit tests, but basically never on real-world code. The same thing applies to source generators as well obviously. /cc @sbomer @agocke @jkoritzinsky
| Author: | vitek-karas |
|---|---|
| Assignees: | - |
| Labels: | `untriaged`, `area-Tools-ILLink`, `needs-area-label` |
| Milestone: | - |
sbomer
jkoritzinsky
Analyzers are compiled against a specific version of the C# compiler. As such they can only recognize the shapes of the code that version of the compiler supports. With a new version of the compiler new shapes become possible, sometimes these are subtle changes, sometimes they mean adding new node types to representation or new enum values to properties. Since we can't update analyzers in sync with the compiler, the analyzer has to be written such that it can survive most of these changes without too much harm (typically it would simply ignore the new shape).
But that means that we don't get to be notified if such a new shape start occurring and can't fix the analyzer unless we have some other reason to look into that case. There will be cases where the analyzer doesn't work the way it should simply because it wasn't updated to match the latest compiler behavior.
We considered adding a warning into the analyzer to ask the user to file an issue if this happens, but that comes with rather severe downsides:
(See the discussion in https://github.com/dotnet/runtime/pull/88836 for some more detail)
For the illink analyzer we decided to change these cases into assertions. But that means we would like to run the debug build of the analyzer on as much code as possible so that we catch these things internally when they happen.
I think it would make sense to do this for all the analyzers where it's possible - there are assertions in their code bases as well which are only exercised on targeted unit tests, but basically never on real-world code.
The same thing applies to source generators as well obviously.
/cc @sbomer @agocke @jkoritzinsky