[ILLink analyzer] Improve handling of invalid operations #94888
Conversation
ghost
added
the
linkable-framework
dotnet-issue-labeler
bot
added
the
area-Tools-ILLink
|
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:
|
| @@ -335,9 +335,6 @@ TValue ProcessSingleTargetAssignment (IOperation targetOperation, IAssignmentOpe | |||
| // Seems like this can't happen with a flow capture operation. | |||
| Debug.Assert (operation.Target is not IFlowCaptureReferenceOperation); | |||
| 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
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