Skip to content

Commit

Permalink
PR Feedback, more testing.
Browse files Browse the repository at this point in the history
  • Loading branch information
333fred committed May 16, 2022
1 parent 7f0369a commit f5b6d07
Show file tree
Hide file tree
Showing 24 changed files with 343 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -299,5 +299,7 @@ internal PEModuleSymbol PrimaryModule

return _lazyCachedCompilerFeatureRequiredDiagnosticInfo;
}

public override bool HasUnsupportedMetadata => GetCompilerFeatureRequiredDiagnostic()?.Code == (int)ErrorCode.ERR_UnsupportedCompilerFeature;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,12 @@ private DiagnosticInfo DeriveCompilerFeatureRequiredDiagnostic()
return diag;
}

diag = Signature.ReturnParam.DeriveCompilerFeatureRequiredDiagnostic(decoder);
if (diag != null)
{
return diag;
}

foreach (var param in Parameters)
{
diag = ((PEParameterSymbol)param).DeriveCompilerFeatureRequiredDiagnostic(decoder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -773,5 +773,7 @@ internal bool ShouldDecodeNullableAttributes(Symbol symbol)

return _lazyCachedCompilerFeatureRequiredDiagnosticInfo ?? (_assemblySymbol as PEAssemblySymbol)?.GetCompilerFeatureRequiredDiagnostic();
}

public override bool HasUnsupportedMetadata => GetCompilerFeatureRequiredDiagnostic()?.Code == (int)ErrorCode.ERR_UnsupportedCompilerFeature;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1072,5 +1072,23 @@ public sealed override bool Equals(Symbol other, TypeCompareKind compareKind)
#nullable enable
internal DiagnosticInfo? DeriveCompilerFeatureRequiredDiagnostic(MetadataDecoder decoder)
=> PEUtilities.DeriveCompilerFeatureRequiredAttributeDiagnostic(this, (PEModuleSymbol)ContainingModule, Handle, allowedFeatures: CompilerFeatureRequiredFeatures.None, decoder);

public override bool HasUnsupportedMetadata
{
get
{
var containingModule = (PEModuleSymbol)ContainingModule;
var decoder = ContainingSymbol switch
{
PEMethodSymbol method => new MetadataDecoder(containingModule, method),
PEPropertySymbol property => new MetadataDecoder(containingModule, (PENamedTypeSymbol)ContainingType),
_ => throw ExceptionUtilities.UnexpectedValue(this.ContainingSymbol.Kind)
};

return DeriveCompilerFeatureRequiredDiagnostic(decoder) is { Code: (int)ErrorCode.ERR_UnsupportedCompilerFeature }
? true
: base.HasUnsupportedMetadata;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -794,12 +794,18 @@ void deriveCompilerFeatureRequiredUseSiteInfo(ref UseSiteInfo<AssemblySymbol> re
{
var containingType = (PENamedTypeSymbol)ContainingType;
PEModuleSymbol containingPEModule = _containingType.ContainingPEModule;
var decoder = new MetadataDecoder(containingPEModule, containingType);
var diag = PEUtilities.DeriveCompilerFeatureRequiredAttributeDiagnostic(
this,
containingPEModule,
Handle,
allowedFeatures: CompilerFeatureRequiredFeatures.None,
new MetadataDecoder(containingPEModule, containingType));
decoder);

foreach (var param in Parameters)
{
diag ??= ((PEParameterSymbol)param).DeriveCompilerFeatureRequiredDiagnostic(decoder);
}

diag ??= containingType.GetCompilerFeatureRequiredDiagnostic();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -709,5 +709,19 @@ private NamedTypeSymbol GetDefaultBaseType()
#nullable enable
internal DiagnosticInfo? DeriveCompilerFeatureRequiredDiagnostic(MetadataDecoder decoder)
=> PEUtilities.DeriveCompilerFeatureRequiredAttributeDiagnostic(this, (PEModuleSymbol)ContainingModule, Handle, CompilerFeatureRequiredFeatures.None, decoder);

public override bool HasUnsupportedMetadata
{
get
{
var containingModule = (PEModuleSymbol)ContainingModule;
MetadataDecoder decoder = ContainingSymbol is PEMethodSymbol method
? new MetadataDecoder(containingModule, method)
: new MetadataDecoder(containingModule, (PENamedTypeSymbol)ContainingSymbol);
return DeriveCompilerFeatureRequiredDiagnostic(decoder)?.Code == (int)ErrorCode.ERR_UnsupportedCompilerFeature
? true
: base.HasUnsupportedMetadata;
}
}
}
}
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/ParameterSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Symbols;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -420,9 +421,9 @@ internal sealed override ObsoleteAttributeData? ObsoleteAttributeData
/// </summary>
internal abstract bool HasInterpolatedStringHandlerArgumentError { get; }

protected sealed override bool IsHighestPriorityUseSiteErrorCode(int code) => code is (int)ErrorCode.ERR_UnsupportedCompilerFeature or (int)ErrorCode.ERR_BindToBogus;
protected sealed override bool IsHighestPriorityUseSiteErrorCode(int code) => code is (int)ErrorCode.ERR_UnsupportedCompilerFeature or (int)ErrorCode.ERR_BogusType;

public sealed override bool HasUnsupportedMetadata
public override bool HasUnsupportedMetadata
{
get
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ public sealed override bool HasUnsupportedMetadata
get
{
DiagnosticInfo info = GetUseSiteInfo().DiagnosticInfo;
return (object)info != null && info.Code == (int)ErrorCode.ERR_BindToBogus;
return (object)info != null && info.Code is (int)ErrorCode.ERR_BindToBogus or (int)ErrorCode.ERR_UnsupportedCompilerFeature;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/Symbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ public virtual bool HasUnsupportedMetadata
{
get
{
return GetUseSiteInfo().DiagnosticInfo?.Code == (int)ErrorCode.ERR_UnsupportedCompilerFeature;
return false;
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -520,15 +520,15 @@ internal Microsoft.Cci.PrimitiveTypeCode PrimitiveTypeCode
/// Returns true if the error code is highest priority while calculating use site error for this symbol.
/// </summary>
protected sealed override bool IsHighestPriorityUseSiteErrorCode(int code)
=> code is (int)ErrorCode.ERR_UnsupportedCompilerFeature or (int)ErrorCode.ERR_BindToBogus or (int)ErrorCode.ERR_BogusType;
=> code is (int)ErrorCode.ERR_UnsupportedCompilerFeature or (int)ErrorCode.ERR_BogusType;


public sealed override bool HasUnsupportedMetadata
public override bool HasUnsupportedMetadata
{
get
{
DiagnosticInfo info = GetUseSiteInfo().DiagnosticInfo;
return (object)info != null && info.Code is (int)ErrorCode.ERR_BogusType or (int)ErrorCode.ERR_UnsupportedCompilerFeature;
return (object)info != null && info.Code is (int)ErrorCode.ERR_UnsupportedCompilerFeature or (int)ErrorCode.ERR_BogusType;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Linq;
using Microsoft.CodeAnalysis.CodeGen;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.UnitTests;
Expand Down Expand Up @@ -83,6 +85,9 @@ protected override void AssertNormalErrors(CSharpCompilation comp)
// (4,10): error CS9512: 'OnMethod.M()' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// OnMethod.M();
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "M").WithArguments("OnMethod.M()", "test").WithLocation(4, 10),
// (5,16): error CS9512: 'void' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// OnMethodReturn.M();
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "M").WithArguments("void", "test").WithLocation(5, 16),
// (6,13): error CS9512: 'int' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// OnParameter.M(1);
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "M").WithArguments("int", "test").WithLocation(6, 13),
Expand Down Expand Up @@ -128,16 +133,105 @@ protected override void AssertNormalErrors(CSharpCompilation comp)
// (26,32): error CS9512: 'int' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// _ = OnIndexedPropertyParameter.get_Property(1);
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "get_Property").WithArguments("int", "test").WithLocation(26, 32),
// (27,29): error CS9512: 'int' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// (27,1): error CS9512: 'int' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// new OnThisIndexerParameter()[1] = 1;
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "[1]").WithArguments("int", "test").WithLocation(27, 29),
// (28,33): error CS9512: 'int' requires compiler feature 'test', which is not supported by this version of the C# compiler.
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "new OnThisIndexerParameter()[1]").WithArguments("int", "test").WithLocation(27, 1),
// (28,5): error CS9512: 'int' requires compiler feature 'test', which is not supported by this version of the C# compiler.
// _ = new OnThisIndexerParameter()[1];
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "[1]").WithArguments("int", "test").WithLocation(28, 33)
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "new OnThisIndexerParameter()[1]").WithArguments("int", "test").WithLocation(28, 5)
);

var onType = comp.GetTypeByMetadataName("OnType");
Assert.True(onType!.HasUnsupportedMetadata);
Assert.True(onType.GetMember<MethodSymbol>("M").HasUnsupportedMetadata);

var onMethod = comp.GetTypeByMetadataName("OnMethod");
Assert.False(onMethod!.HasUnsupportedMetadata);
Assert.True(onMethod.GetMember<MethodSymbol>("M").HasUnsupportedMetadata);

var onMethodReturn = comp.GetTypeByMetadataName("OnMethodReturn");
Assert.False(onMethodReturn!.HasUnsupportedMetadata);
Assert.True(onMethodReturn.GetMember<MethodSymbol>("M").HasUnsupportedMetadata);

var onParameter = comp.GetTypeByMetadataName("OnParameter");
Assert.False(onParameter!.HasUnsupportedMetadata);
var onParameterMethod = onParameter.GetMember<MethodSymbol>("M");
Assert.True(onParameterMethod.HasUnsupportedMetadata);
Assert.True(onParameterMethod.Parameters[0].HasUnsupportedMetadata);

var onField = comp.GetTypeByMetadataName("OnField");
Assert.False(onField!.HasUnsupportedMetadata);
Assert.True(onField.GetMember<FieldSymbol>("Field").HasUnsupportedMetadata);

var onProperty = comp.GetTypeByMetadataName("OnProperty");
Assert.False(onProperty!.HasUnsupportedMetadata);
Assert.True(onProperty.GetMember<PropertySymbol>("Property").HasUnsupportedMetadata);

var onPropertyGetter = comp.GetTypeByMetadataName("OnPropertyGetter");
Assert.False(onPropertyGetter!.HasUnsupportedMetadata);
var onPropertyGetterProperty = onPropertyGetter.GetMember<PropertySymbol>("Property");
Assert.False(onPropertyGetterProperty.HasUnsupportedMetadata);
Assert.False(onPropertyGetterProperty.SetMethod.HasUnsupportedMetadata);
Assert.True(onPropertyGetterProperty.GetMethod.HasUnsupportedMetadata);

var onPropertySetter = comp.GetTypeByMetadataName("OnPropertySetter");
Assert.False(onPropertySetter!.HasUnsupportedMetadata);
var onPropertySetterProperty = onPropertySetter.GetMember<PropertySymbol>("Property");
Assert.False(onPropertySetterProperty.HasUnsupportedMetadata);
Assert.True(onPropertySetterProperty.SetMethod.HasUnsupportedMetadata);
Assert.False(onPropertySetterProperty.GetMethod.HasUnsupportedMetadata);

var onEvent = comp.GetTypeByMetadataName("OnEvent");
Assert.False(onEvent!.HasUnsupportedMetadata);
Assert.True(onEvent.GetMember<EventSymbol>("Event").HasUnsupportedMetadata);

var onEventAdder = comp.GetTypeByMetadataName("OnEventAdder");
Assert.False(onEventAdder!.HasUnsupportedMetadata);
var onEventAdderEvent = onEventAdder.GetMember<EventSymbol>("Event");
Assert.False(onEventAdderEvent.HasUnsupportedMetadata);
Assert.True(onEventAdderEvent.AddMethod!.HasUnsupportedMetadata);
Assert.False(onEventAdderEvent.RemoveMethod!.HasUnsupportedMetadata);

var onEventRemover = comp.GetTypeByMetadataName("OnEventRemover");
Assert.False(onEventRemover!.HasUnsupportedMetadata);
var onEventRemoverEvent = onEventRemover.GetMember<EventSymbol>("Event");
Assert.False(onEventRemoverEvent.HasUnsupportedMetadata);
Assert.False(onEventRemoverEvent.AddMethod!.HasUnsupportedMetadata);
Assert.True(onEventRemoverEvent.RemoveMethod!.HasUnsupportedMetadata);

var onEnum = comp.GetTypeByMetadataName("OnEnum");
Assert.True(onEnum!.HasUnsupportedMetadata);

var onEnumMember = comp.GetTypeByMetadataName("OnEnumMember");
Assert.False(onEnumMember!.HasUnsupportedMetadata);
Assert.True(onEnumMember.GetMember<FieldSymbol>("A").HasUnsupportedMetadata);

var onClassTypeParameter = comp.GetTypeByMetadataName("OnClassTypeParameter`1");
Assert.True(onClassTypeParameter!.HasUnsupportedMetadata);
Assert.True(onClassTypeParameter.TypeParameters[0].HasUnsupportedMetadata);

var onMethodTypeParameter = comp.GetTypeByMetadataName("OnMethodTypeParameter");
Assert.False(onMethodTypeParameter!.HasUnsupportedMetadata);
var onMethodTypeParameterMethod = onMethodTypeParameter.GetMember<MethodSymbol>("M");
Assert.True(onMethodTypeParameterMethod.HasUnsupportedMetadata);
Assert.True(onMethodTypeParameterMethod.TypeParameters[0].HasUnsupportedMetadata);

var onDelegateType = comp.GetTypeByMetadataName("OnDelegateType");
Assert.True(onDelegateType!.HasUnsupportedMetadata);

var onIndexedPropertyParameter = comp.GetTypeByMetadataName("OnIndexedPropertyParameter");
Assert.False(onIndexedPropertyParameter!.HasUnsupportedMetadata);
Assert.True(onIndexedPropertyParameter.GetMember<MethodSymbol>("get_Property").Parameters[0].HasUnsupportedMetadata);
Assert.True(onIndexedPropertyParameter.GetMember<MethodSymbol>("set_Property").Parameters[0].HasUnsupportedMetadata);

var onThisParameterIndexer = comp.GetTypeByMetadataName("OnThisIndexerParameter");
Assert.False(onThisParameterIndexer!.HasUnsupportedMetadata);
var indexer = onThisParameterIndexer.GetMember<PropertySymbol>("this[]");
Assert.True(indexer.HasUnsupportedMetadata);
Assert.True(indexer.Parameters[0].HasUnsupportedMetadata);
}

protected override void AssertModuleErrors(CSharpCompilation comp)
protected override void AssertModuleErrors(CSharpCompilation comp, MetadataReference ilRef)
{
comp.VerifyDiagnostics(
// (2,1): error CS9512: 'OnModule' requires compiler feature 'test', which is not supported by this version of the C# compiler.
Expand Down Expand Up @@ -291,9 +385,11 @@ protected override void AssertModuleErrors(CSharpCompilation comp)
// _ = new OnThisIndexerParameter()[1];
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "OnThisIndexerParameter").WithArguments("OnModule", "test").WithLocation(28, 9)
);

Assert.True(comp.GetReferencedAssemblySymbol(ilRef).Modules.Single().HasUnsupportedMetadata);
}

protected override void AssertAssemblyErrors(CSharpCompilation comp)
protected override void AssertAssemblyErrors(CSharpCompilation comp, MetadataReference ilRef)
{
comp.VerifyDiagnostics(
// (2,1): error CS9512: 'AssemblyTest, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' requires compiler feature 'test', which is not supported by this version of the C# compiler.
Expand Down Expand Up @@ -447,6 +543,8 @@ protected override void AssertAssemblyErrors(CSharpCompilation comp)
// _ = new OnThisIndexerParameter()[1];
Diagnostic(ErrorCode.ERR_UnsupportedCompilerFeature, "OnThisIndexerParameter").WithArguments("AssemblyTest, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "test").WithLocation(28, 9)
);

Assert.True(comp.GetReferencedAssemblySymbol(ilRef).HasUnsupportedMetadata);
}

[Fact]
Expand Down
11 changes: 6 additions & 5 deletions src/Compilers/Test/Core/BaseCompilerFeatureRequiredTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -672,11 +672,11 @@ .assembly extern mscorlib
}
else
{
AssertAssemblyErrors(comp);
AssertAssemblyErrors(comp, compiledIl);
}
}

protected abstract void AssertAssemblyErrors(TCompilation compilation);
protected abstract void AssertAssemblyErrors(TCompilation compilation, MetadataReference ilRef);

[Theory]
[InlineData(true)]
Expand All @@ -693,17 +693,18 @@ .module OnModule
""";


var comp = CreateCompilationWithIL(source: GetUsage(), ilSource: il);
var compiledIl = CompileIL(il);
var comp = CreateCompilation(source: GetUsage(), references: new[] { compiledIl });

if (isOptional == true)
{
CompileAndVerify(comp).VerifyDiagnostics();
}
else
{
AssertModuleErrors(comp);
AssertModuleErrors(comp, compiledIl);
}
}

protected abstract void AssertModuleErrors(TCompilation compilation);
protected abstract void AssertModuleErrors(TCompilation compilation, MetadataReference ilRef);
}
Original file line number Diff line number Diff line change
Expand Up @@ -271,5 +271,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE

Return _lazyCachedCompilerFeatureRequiredDiagnosticInfo
End Function

Public Overrides ReadOnly Property HasUnsupportedMetadata As Boolean
Get
Dim info = GetCompilerFeatureRequiredDiagnosticInfo()
If info IsNot Nothing Then
Return info.Code = DirectCast(ERRID.ERR_UnsupportedCompilerFeature, Integer)
End If

Return False
End Get
End Property
End Class
End Namespace
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE
Return
End If

errorInfo = Signature.ReturnParam.DeriveCompilerFeatureRequiredDiagnostic(decoder)
If errorInfo IsNot Nothing Then
Return
End If

For Each parameter In Parameters
errorInfo = DirectCast(parameter, PEParameterSymbol).DeriveCompilerFeatureRequiredDiagnostic(decoder)
If errorInfo IsNot Nothing Then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,5 +498,16 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols.Metadata.PE
Return If(_lazyCachedCompilerFeatureRequiredDiagnosticInfo,
TryCast(ContainingAssembly, PEAssemblySymbol)?.GetCompilerFeatureRequiredDiagnosticInfo())
End Function

Public Overrides ReadOnly Property HasUnsupportedMetadata As Boolean
Get
Dim info = GetCompilerFeatureRequiredDiagnostic()
If info IsNot Nothing Then
Return info.Code = DirectCast(ERRID.ERR_UnsupportedCompilerFeature, Integer)
End If

Return False
End Get
End Property
End Class
End Namespace
Loading

0 comments on commit f5b6d07

Please sign in to comment.