Skip to content

Commit

Permalink
Don't crash the trim analyzer if it finds unrecognized nodes in the i…
Browse files Browse the repository at this point in the history
…nput (#88836)

New versions of the compiler will introduce new nodes and values. The analyzer can never be 100% in sycn with the compiler, so it needs to be able to gracefully handle nodes it doesn't know anything about.

Change the several throws to just Debug.Fail. For end-users if we hit unrecognized node, we will effectively ignore that part of the code. So not 100% precise, but the analyzer will never be 100% regardles.

This is in response to #88684, but we can't add tests for it yet because the necessary compiler changes are in Preview 6, the repo is still on Preview 5.
  • Loading branch information
vitek-karas committed Jul 17, 2023
1 parent 84e09f9 commit 0f56e16
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Diagnostics;
using ILLink.Shared.DataFlow;
using Microsoft.CodeAnalysis;

Expand Down Expand Up @@ -29,7 +30,12 @@ public CapturedReferenceValue (IOperation operation)
// These will just be ignored when referenced later.
break;
default:
throw new NotImplementedException (operation.Kind.ToString ());
// Assert on anything else as it means we need to implement support for it
// 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 ($"{operation.GetType ()}: {operation.Syntax.GetLocation ().GetLineSpan ()}");
break;
}
Reference = operation;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,13 @@ public override TValue VisitSimpleAssignment (ISimpleAssignmentOperation operati
// (don't have specific I*Operation types), such as pointer dereferences.
if (targetOperation.Kind is OperationKind.None)
break;
throw new NotImplementedException ($"{targetOperation.GetType ()}: {targetOperation.Syntax.GetLocation ().GetLineSpan ()}");

// Assert on anything else as it means we need to implement support for it
// 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 ()}");
break;
}
return Visit (operation.Value, state);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Diagnostics;
using ILLink.RoslynAnalyzer;
using Microsoft.CodeAnalysis;

Expand All @@ -15,18 +15,23 @@ public ParameterProxy (IParameterSymbol parameter)
Index = (ParameterIndex) parameter.Ordinal + (Method.HasImplicitThis () ? 1 : 0);
}

public partial ReferenceKind GetReferenceKind () =>
IsImplicitThis
? Method.Method.ContainingType.IsValueType
? ReferenceKind.Ref
: ReferenceKind.None
: Method.Method.Parameters[MetadataIndex].RefKind switch {
RefKind.Ref => ReferenceKind.Ref,
RefKind.In => ReferenceKind.In,
RefKind.Out => ReferenceKind.Out,
RefKind.None => ReferenceKind.None,
_ => throw new NotImplementedException ($"Unexpected RefKind found on parameter {GetDisplayName ()}")
};
public partial ReferenceKind GetReferenceKind ()
{
if (IsImplicitThis)
return Method.Method.ContainingType.IsValueType
? ReferenceKind.Ref
: ReferenceKind.None;

switch (Method.Method.Parameters[MetadataIndex].RefKind) {
case RefKind.Ref: return ReferenceKind.Ref;
case RefKind.In: return ReferenceKind.In;
case RefKind.Out: return ReferenceKind.Out;
case RefKind.None: return ReferenceKind.None;
default:
Debug.Fail ($"Unexpected RefKind {Method.Method.Parameters[MetadataIndex].RefKind} found on parameter {GetDisplayName ()}");
return ReferenceKind.None;
}
}

/// <summary>
/// Returns the IParameterSymbol representing the parameter. Returns null for the implicit this paramter.
Expand Down

0 comments on commit 0f56e16

Please sign in to comment.