[ILLink analyzer] Improve handling of invalid operations #94888
Conversation
ghost
added
linkable-framework
|
Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar Issue DetailsAdds a helper that's used in the default case whenever we switch on the IOperation type, that:
|
| break; | ||
| case IInvalidOperation: | ||
| // This can happen for a field assignment in an attribute instance. | ||
| // TODO: validate against the field attributes. |
There was a problem hiding this comment.
I checked that the current behavior doesn't give IInvalidOperation for field assignments in attribute instances - possibly fixed by some recent work that touched the way we do attribute analysis. While I was touching AttributeFieldDataflow I cleaned it up a bit, along with the similar testcase AttributePropertyDataflow.
| // but do not throw here as it means new Roslyn version could cause the analyzer to crash | ||
| // which is not fixable by the user. The analyzer is not going to be 100% correct no matter what we do | ||
| // so effectively ignoring constructs it doesn't understand is OK. | ||
| Debug.Fail ($"{targetOperation.GetType ()}: {targetOperation.Syntax.GetLocation ().GetLineSpan ()}"); |
There was a problem hiding this comment.
Can we use release builds of the analyzer in local development? Such asserts make it really hard to continue working when they occur. Or something else besides an assert.
There was a problem hiding this comment.
I changed back to throwing like we used to, but only in Debug builds. This should surface as AD0001 warnings, which can be silenced if needed, while still giving us feedback on any analyzer crashes. Related to the discussion in #90358.
This should produce AD0001 instead
vitek-karas
left a comment
vitek-karas
left a comment
There was a problem hiding this comment.
Looks good - thanks a lot!
3 participants
Adds a helper that's used in the default case whenever we switch on the IOperation type, that:
assertsthrows otherwise, in Debug modeFixes #94858
Fixes #94260