Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VB: Recognize and check an unmanaged constraint #75665

Merged
merged 5 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 5 additions & 57 deletions src/Compilers/CSharp/Portable/Symbols/BaseTypeAnalysis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Diagnostics;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Symbols;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
Expand Down Expand Up @@ -122,7 +123,9 @@ private static void StructDependsClosure(NamedTypeSymbol type, HashSet<Symbol> p
/// </summary>
internal static ManagedKind GetManagedKind(NamedTypeSymbol type, ref CompoundUseSiteInfo<AssemblySymbol> useSiteInfo)
{
var (isManaged, hasGenerics) = IsManagedTypeHelper(type);
// The code below should be kept in sync with NamedTypeSymbol.GetManagedKind in VB

var (isManaged, hasGenerics) = INamedTypeSymbolInternal.Helpers.IsManagedTypeHelper(type);
var definitelyManaged = isManaged == ThreeState.True;
if (isManaged == ThreeState.Unknown)
{
Expand Down Expand Up @@ -201,7 +204,7 @@ internal static ManagedKind GetManagedKind(NamedTypeSymbol type, ref CompoundUse
}
else
{
var result = IsManagedTypeHelper(fieldNamedType);
var result = INamedTypeSymbolInternal.Helpers.IsManagedTypeHelper(fieldNamedType);
hasGenerics = hasGenerics || result.hasGenerics;
// NOTE: don't use ManagedKind.get on a NamedTypeSymbol - that could lead
// to infinite recursion.
Expand Down Expand Up @@ -235,60 +238,5 @@ internal static ManagedKind GetManagedKind(NamedTypeSymbol type, ref CompoundUse

internal static TypeSymbol NonPointerType(this FieldSymbol field) =>
field.HasPointerType ? null : field.Type;

/// <summary>
/// Returns True or False if we can determine whether the type is managed
/// without looking at its fields and Unknown otherwise.
/// Also returns whether or not the given type is generic.
/// </summary>
private static (ThreeState isManaged, bool hasGenerics) IsManagedTypeHelper(NamedTypeSymbol type)
{
// To match dev10, we treat enums as their underlying types.
if (type.IsEnumType())
{
type = type.GetEnumUnderlyingType();
}

// Short-circuit common cases.
switch (type.SpecialType)
{
case SpecialType.System_Void:
case SpecialType.System_Boolean:
case SpecialType.System_Char:
case SpecialType.System_SByte:
case SpecialType.System_Byte:
case SpecialType.System_Int16:
case SpecialType.System_UInt16:
case SpecialType.System_Int32:
case SpecialType.System_UInt32:
case SpecialType.System_Int64:
case SpecialType.System_UInt64:
case SpecialType.System_Decimal:
case SpecialType.System_Single:
case SpecialType.System_Double:
case SpecialType.System_IntPtr:
case SpecialType.System_UIntPtr:
case SpecialType.System_ArgIterator:
case SpecialType.System_RuntimeArgumentHandle:
return (ThreeState.False, false);
case SpecialType.System_TypedReference:
return (ThreeState.True, false);
case SpecialType.None:
default:
// CONSIDER: could provide cases for other common special types.
break; // Proceed with additional checks.
}

bool hasGenerics = type.IsGenericType;
switch (type.TypeKind)
{
case TypeKind.Enum:
return (ThreeState.False, hasGenerics);
case TypeKind.Struct:
return (ThreeState.Unknown, hasGenerics);
default:
return (ThreeState.True, hasGenerics);
}
}
}
}
15 changes: 15 additions & 0 deletions src/Compilers/Core/Portable/Binding/UseSiteInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,21 @@ public void AddDiagnostics(ImmutableArray<DiagnosticInfo> diagnostics)
}
}

public void AddDiagnosticInfo(DiagnosticInfo diagnosticInfo)
{
if (!AccumulatesDiagnostics)
{
return;
}

_diagnostics ??= new HashSet<DiagnosticInfo>();

if (_diagnostics.Add(diagnosticInfo) && diagnosticInfo?.Severity == DiagnosticSeverity.Error)
{
RecordPresenceOfAnError();
}
Copy link
Member

@cston cston Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is repeated several times. Consider extracting a helper method. #Resolved

}

public void AddDependencies(UseSiteInfo<TAssemblySymbol> info)
{
if (!_hasErrors && AccumulatesDependencies)
Expand Down
65 changes: 65 additions & 0 deletions src/Compilers/Core/Portable/Symbols/INamedTypeSymbolInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Diagnostics;

namespace Microsoft.CodeAnalysis.Symbols
{
Expand All @@ -16,5 +17,69 @@ internal interface INamedTypeSymbolInternal : ITypeSymbolInternal

ImmutableArray<ISymbolInternal> GetMembers();
ImmutableArray<ISymbolInternal> GetMembers(string name);

/// <summary>
/// True if this type or some containing type has type parameters.
/// </summary>
public bool IsGenericType { get; }
Copy link
Member

@cston cston Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public

Is the public qualifier needed? #Resolved


internal static class Helpers
{
/// <summary>
/// Returns True or False if we can determine whether the type is managed
/// without looking at its fields and Unknown otherwise.
/// Also returns whether or not the given type is generic.
/// </summary>
public static (ThreeState isManaged, bool hasGenerics) IsManagedTypeHelper(INamedTypeSymbolInternal type)
Copy link
Member

@cston cston Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsManagedTypeHelper

Consider use a top-level class and make this an extension method. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider use a top-level class and make this an extension method.

I prefer to keep the code as is. I do not foresee us using this helper much in the future. Therefore, in my opinion there is no good reason to "pollute" name lookup binding space with it. If we could use Default Interface Implementations feature in our code, I would make this method a member of INamedTypeSymbolInternal.

{
// To match dev10, we treat enums as their underlying types.
if (type.TypeKind == TypeKind.Enum)
{
Debug.Assert(type.EnumUnderlyingType is not null);
type = type.EnumUnderlyingType;
}

// Short-circuit common cases.
switch (type.SpecialType)
{
case SpecialType.System_Void:
case SpecialType.System_Boolean:
case SpecialType.System_Char:
case SpecialType.System_SByte:
case SpecialType.System_Byte:
case SpecialType.System_Int16:
case SpecialType.System_UInt16:
case SpecialType.System_Int32:
case SpecialType.System_UInt32:
case SpecialType.System_Int64:
case SpecialType.System_UInt64:
case SpecialType.System_Decimal:
case SpecialType.System_Single:
case SpecialType.System_Double:
case SpecialType.System_IntPtr:
case SpecialType.System_UIntPtr:
case SpecialType.System_ArgIterator:
case SpecialType.System_RuntimeArgumentHandle:
return (ThreeState.False, false);
case SpecialType.System_TypedReference:
return (ThreeState.True, false);
case SpecialType.None:
default:
// CONSIDER: could provide cases for other common special types.
break; // Proceed with additional checks.
}

bool hasGenerics = type.IsGenericType;
switch (type.TypeKind)
{
case TypeKind.Enum:
return (ThreeState.False, hasGenerics);
case TypeKind.Struct:
return (ThreeState.Unknown, hasGenerics);
default:
return (ThreeState.True, hasGenerics);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public void BasicPathMapWindows(string filePath, string workingDirectory, string
""specifiedLanguageVersion"": ""VisualBasic15"",
""preprocessorSymbols"": {
""TARGET"": ""exe"",
""VBC_VER"": ""16.9""
""VBC_VER"": ""17.13""
}
}
}
Expand Down Expand Up @@ -360,7 +360,7 @@ public void FeatureFlag()
""specifiedLanguageVersion"": ""Default"",
""preprocessorSymbols"": {{
""TARGET"": ""library"",
""VBC_VER"": ""16.9"",
""VBC_VER"": ""17.13"",
""_MYTYPE"": ""Empty""
}}
}}
Expand All @@ -385,7 +385,7 @@ public void FeatureFlag()
""specifiedLanguageVersion"": ""Default"",
""preprocessorSymbols"": {{
""TARGET"": ""library"",
""VBC_VER"": ""16.9"",
""VBC_VER"": ""17.13"",
""_MYTYPE"": ""Empty""
}}
}}
Expand Down
9 changes: 7 additions & 2 deletions src/Compilers/Test/Utilities/VisualBasic/BasicTestSource.vb
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,17 @@ Public Structure BasicTestSource

Dim source = TryCast(Value, String)
If source IsNot Nothing Then
Return New SyntaxTree() {VisualBasicSyntaxTree.ParseText(SourceText.From(source, encoding:=Nothing, SourceHashAlgorithms.Default), parseOptions)}
Return New SyntaxTree() _
{
VisualBasicSyntaxTree.ParseText(
SourceText.From(source, encoding:=Nothing, SourceHashAlgorithms.Default),
If(parseOptions, TestOptions.RegularLatest))
}
End If

Dim sources = TryCast(Value, String())
If sources IsNot Nothing Then
Return sources.Select(Function(s) VisualBasicSyntaxTree.ParseText(SourceText.From(s, encoding:=Nothing, SourceHashAlgorithms.Default), parseOptions)).ToArray()
Return sources.Select(Function(s) VisualBasicSyntaxTree.ParseText(SourceText.From(s, encoding:=Nothing, SourceHashAlgorithms.Default), If(parseOptions, TestOptions.RegularLatest))).ToArray()
End If

Dim tree = TryCast(Value, SyntaxTree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ Friend Module CompilationUtils
MarkupTestFile.GetSpans(codeWithMarker, codeWithoutMarker, spans)

Dim text = SourceText.From(codeWithoutMarker, Encoding.UTF8)
Return (VisualBasicSyntaxTree.ParseText(text, parseOptions, If(programElement.@name, "")), spans)
Return (VisualBasicSyntaxTree.ParseText(text, If(parseOptions, TestOptions.RegularLatest), If(programElement.@name, "")), spans)
End Function

' Find a node inside a tree.
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/Test/Utilities/VisualBasic/Extensions.vb
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,12 @@ Friend Module Extensions

<Extension>
Friend Function ReduceExtensionMethod(this As MethodSymbol, instanceType As TypeSymbol) As MethodSymbol
Return this.ReduceExtensionMethod(instanceType, CompoundUseSiteInfo(Of AssemblySymbol).Discarded)
Return this.ReduceExtensionMethod(instanceType, CompoundUseSiteInfo(Of AssemblySymbol).Discarded, LanguageVersion.Latest)
End Function

<Extension>
Friend Function ReduceExtensionMethod(this As MethodSymbol, instanceType As TypeSymbol, proximity As Integer) As MethodSymbol
Return this.ReduceExtensionMethod(instanceType, proximity, CompoundUseSiteInfo(Of AssemblySymbol).Discarded)
Return this.ReduceExtensionMethod(instanceType, proximity, CompoundUseSiteInfo(Of AssemblySymbol).Discarded, LanguageVersion.Latest)
End Function

<Extension>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ Friend Module ParserTestUtilities
encoding = Encoding.UTF8
End If

Dim tree = VisualBasicSyntaxTree.ParseText(SourceText.From(source, encoding), options:=If(options, VisualBasicParseOptions.Default), path:=fileName)
Dim tree = VisualBasicSyntaxTree.ParseText(SourceText.From(source, encoding), options:=If(options, TestOptions.RegularLatest), path:=fileName)
Dim root = tree.GetRoot()
' Verify FullText
Assert.Equal(source, root.ToFullString)
Expand Down
1 change: 1 addition & 0 deletions src/Compilers/Test/Utilities/VisualBasic/TestOptions.vb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Public Class TestOptions
Public Shared ReadOnly Regular15_5 As VisualBasicParseOptions = Regular.WithLanguageVersion(LanguageVersion.VisualBasic15_5)
Public Shared ReadOnly Regular16 As VisualBasicParseOptions = Regular.WithLanguageVersion(LanguageVersion.VisualBasic16)
Public Shared ReadOnly Regular16_9 As VisualBasicParseOptions = Regular.WithLanguageVersion(LanguageVersion.VisualBasic16_9)
Public Shared ReadOnly Regular17_13 As VisualBasicParseOptions = Regular.WithLanguageVersion(LanguageVersion.VisualBasic17_13)
Public Shared ReadOnly RegularLatest As VisualBasicParseOptions = Regular.WithLanguageVersion(LanguageVersion.Latest)
Public Shared ReadOnly RegularWithLegacyStrongName As VisualBasicParseOptions = Regular.WithFeature("UseLegacyStrongNameProvider")

Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/VisualBasic/Portable/Binding/Binder.vb
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
Select Case Me.BindingLocation
Case BindingLocation.BaseTypes,
BindingLocation.MethodSignature,
BindingLocation.FieldType,
BindingLocation.PropertyType,
BindingLocation.EventType,
BindingLocation.GenericConstraintsClause,
BindingLocation.ProjectImportsDeclaration,
BindingLocation.SourceFileImportsDeclaration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2274,7 +2274,7 @@ ProduceBoundNode:
Dim method = DirectCast(candidate.UnderlyingSymbol, MethodSymbol)
' TODO: Dev10 uses the location of the type parameter or argument that
' violated the constraint, rather than the entire invocation expression.
Dim succeeded = method.CheckConstraints(diagnosticLocation, diagnostics, template:=GetNewCompoundUseSiteInfo(diagnostics))
Dim succeeded = method.CheckConstraints(Compilation.LanguageVersion, diagnosticLocation, diagnostics, template:=GetNewCompoundUseSiteInfo(diagnostics))
Debug.Assert(Not succeeded)
Return
End If
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
' Process all methods from the same type together.
Do
' Try to reduce this method and merge with the current result
Dim reduced As MethodSymbol = methods(i).ReduceExtensionMethod(container, proximity, useSiteInfo)
Dim reduced As MethodSymbol = methods(i).ReduceExtensionMethod(container, proximity, useSiteInfo, binder.Compilation.LanguageVersion)

If reduced IsNot Nothing Then
lookupResult.MergeOverloadedOrPrioritizedExtensionMethods(binder.CheckViability(reduced, arity, options, reduced.ContainingType, useSiteInfo))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
' Interface IA(Of T As C) : End Interface
' Interface IB(Of T As C) : Inherits IA(Of T) : End Interface
If checkConstraints AndAlso ShouldCheckConstraints Then
constructedType.CheckConstraintsForNonTuple(syntaxArguments, diagnostics, template:=GetNewCompoundUseSiteInfo(diagnostics))
constructedType.CheckConstraintsForNonTuple(Compilation.LanguageVersion, syntaxArguments, diagnostics, template:=GetNewCompoundUseSiteInfo(diagnostics))
End If

constructedType = DirectCast(TupleTypeSymbol.TransformToTupleIfCompatible(constructedType), NamedTypeSymbol)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
If ShouldCheckConstraints Then
Dim diagnosticsBuilder = ArrayBuilder(Of TypeParameterDiagnosticInfo).GetInstance()
Dim useSiteDiagnosticsBuilder As ArrayBuilder(Of TypeParameterDiagnosticInfo) = Nothing
constructedType.CheckConstraints(diagnosticsBuilder, useSiteDiagnosticsBuilder, template:=GetNewCompoundUseSiteInfo(diagBag))
constructedType.CheckConstraints(Compilation.LanguageVersion, diagnosticsBuilder, useSiteDiagnosticsBuilder, template:=GetNewCompoundUseSiteInfo(diagBag))

If useSiteDiagnosticsBuilder IsNot Nothing Then
diagnosticsBuilder.AddRange(useSiteDiagnosticsBuilder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
EventSignature
FieldType
HandlesClause
PropertyType
PropertySignature
PropertyAccessorSignature
EventType
EventAccessorSignature
End Enum

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Emit
Debug.Assert(type.AllowsRefLikeType = other.AllowsRefLikeType)
Debug.Assert(type.HasReferenceTypeConstraint = other.HasReferenceTypeConstraint)
Debug.Assert(type.ConstraintTypesNoUseSiteDiagnostics.Length = other.ConstraintTypesNoUseSiteDiagnostics.Length)
Debug.Assert(type.HasUnmanagedTypeConstraint = other.HasUnmanagedTypeConstraint)
Return True
End Function

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/VisualBasic/Portable/Errors/ErrorFacts.vb
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
ERRID.ERR_DoNotUseRequiredMember,
ERRID.ERR_UnsupportedRefReturningCallInWithStatement,
ERRID.ERR_TypeReserved,
ERRID.ERR_UnmanagedConstraintNotSatisfied,
ERRID.ERR_NextAvailable,
ERRID.WRN_UseOfObsoleteSymbol2,
ERRID.WRN_InvalidOverrideDueToTupleNames2,
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/VisualBasic/Portable/Errors/Errors.vb
Original file line number Diff line number Diff line change
Expand Up @@ -1781,8 +1781,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
ERR_LockTypeUnsupported = 37329
ERR_InvalidVersionFormatDeterministic = 37330
ERR_TypeReserved = 37331
ERR_UnmanagedConstraintNotSatisfied = 37332

ERR_NextAvailable = 37332
ERR_NextAvailable = 37333

'// WARNINGS BEGIN HERE
WRN_UseOfObsoleteSymbol2 = 40000
Expand Down Expand Up @@ -2081,5 +2082,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
FEATURE_CommentsAfterLineContinuation
FEATURE_InitOnlySettersUsage
FEATURE_CallerArgumentExpression
FEATURE_UnmanagedConstraint
End Enum
End Namespace
Loading
Loading