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

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 dotnet#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 13, 2023
1 parent e4c5b3e commit 0a3cded
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 0a3cded

Please sign in to comment.