Skip to content

Commit

Permalink
Ensure errors are reported when compiler is unable to emit a required…
Browse files Browse the repository at this point in the history
… attribute. (#70556)

Related to #70338.
  • Loading branch information
AlekseyTs authored Oct 26, 2023
1 parent 01f7aaf commit 06072fb
Show file tree
Hide file tree
Showing 24 changed files with 552 additions and 154 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ internal override int GetTargetAttributeSignatureIndex(CSharpAttributeData attrD
return attrData.GetTargetAttributeSignatureIndex(description);
}

internal override CSharpAttributeData CreateSynthesizedAttribute(WellKnownMember constructor, CSharpAttributeData attrData, SyntaxNode syntaxNodeOpt, DiagnosticBag diagnostics)
internal override CSharpAttributeData CreateSynthesizedAttribute(WellKnownMember constructor, ImmutableArray<TypedConstant> constructorArguments, ImmutableArray<KeyValuePair<string, TypedConstant>> namedArguments, SyntaxNode syntaxNodeOpt, DiagnosticBag diagnostics)
{
var ctor = GetWellKnownMethod(constructor, syntaxNodeOpt, diagnostics);
if ((object)ctor == null)
Expand All @@ -136,7 +136,7 @@ internal override CSharpAttributeData CreateSynthesizedAttribute(WellKnownMember
// the original source interface as both source interface and event provider. Otherwise, we'd have to embed
// the event provider class too.
return SynthesizedAttributeData.Create(ModuleBeingBuilt.Compilation, ctor,
ImmutableArray.Create<TypedConstant>(attrData.CommonConstructorArguments[0], attrData.CommonConstructorArguments[0]),
ImmutableArray.Create<TypedConstant>(constructorArguments[0], constructorArguments[0]),
ImmutableArray<KeyValuePair<string, TypedConstant>>.Empty);

case WellKnownMember.System_Runtime_InteropServices_CoClassAttribute__ctor:
Expand All @@ -149,10 +149,26 @@ internal override CSharpAttributeData CreateSynthesizedAttribute(WellKnownMember
ImmutableArray<KeyValuePair<string, TypedConstant>>.Empty);

default:
return SynthesizedAttributeData.Create(ModuleBeingBuilt.Compilation, ctor, attrData.CommonConstructorArguments, attrData.CommonNamedArguments);
return SynthesizedAttributeData.Create(ModuleBeingBuilt.Compilation, ctor, constructorArguments, namedArguments);
}
}

internal override bool TryGetAttributeArguments(CSharpAttributeData attrData, out ImmutableArray<TypedConstant> constructorArguments, out ImmutableArray<KeyValuePair<string, TypedConstant>> namedArguments, SyntaxNode syntaxNodeOpt, DiagnosticBag diagnostics)
{
bool result = !attrData.HasErrors;

constructorArguments = attrData.CommonConstructorArguments;
namedArguments = attrData.CommonNamedArguments;

DiagnosticInfo errorInfo = attrData.ErrorInfo;
if (errorInfo is not null)
{
diagnostics.Add(errorInfo, syntaxNodeOpt?.Location ?? NoLocation.Singleton);
}

return result;
}

internal string GetAssemblyGuidString(AssemblySymbol assembly)
{
Debug.Assert(!IsFrozen); // After we freeze the set of types, we might add additional assemblies into this map without actual guid values.
Expand Down
27 changes: 14 additions & 13 deletions src/Compilers/CSharp/Portable/Symbols/Attributes/AttributeData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ internal abstract partial class CSharpAttributeData : AttributeData
[MemberNotNullWhen(true, nameof(AttributeClass), nameof(AttributeConstructor))]
internal abstract override bool HasErrors { get; }

internal abstract DiagnosticInfo? ErrorInfo { get; }

internal abstract override bool IsConditionallyOmitted { get; }

/// <summary>
Expand Down Expand Up @@ -99,8 +101,6 @@ internal bool IsSecurityAttribute(CSharpCompilation compilation)
{
if (_lazyIsSecurityAttribute == ThreeState.Unknown)
{
Debug.Assert(!this.HasErrors);

// CLI spec (Partition II Metadata), section 21.11 "DeclSecurity : 0x0E" states:
// SPEC: If the attribute's type is derived (directly or indirectly) from System.Security.Permissions.SecurityAttribute then
// SPEC: it is a security custom attribute and requires special treatment.
Expand All @@ -110,12 +110,18 @@ internal bool IsSecurityAttribute(CSharpCompilation compilation)
// NOTE: We will follow the specification.
// NOTE: See Devdiv Bug #13762 "Custom security attributes deriving from SecurityAttribute are not treated as security attributes" for details.

// Well-known type SecurityAttribute is optional.
// Native compiler doesn't generate a use-site error if it is not found, we do the same.
var wellKnownType = compilation.GetWellKnownType(WellKnownType.System_Security_Permissions_SecurityAttribute);
Debug.Assert(AttributeClass is object);
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
_lazyIsSecurityAttribute = AttributeClass.IsDerivedFrom(wellKnownType, TypeCompareKind.ConsiderEverything, useSiteInfo: ref discardedUseSiteInfo).ToThreeState();
if (AttributeClass is object)
{
// Well-known type SecurityAttribute is optional.
// Native compiler doesn't generate a use-site error if it is not found, we do the same.
var wellKnownType = compilation.GetWellKnownType(WellKnownType.System_Security_Permissions_SecurityAttribute);
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
_lazyIsSecurityAttribute = AttributeClass.IsDerivedFrom(wellKnownType, TypeCompareKind.ConsiderEverything, useSiteInfo: ref discardedUseSiteInfo).ToThreeState();
}
else
{
_lazyIsSecurityAttribute = ThreeState.False;
}
}

return _lazyIsSecurityAttribute.Value();
Expand Down Expand Up @@ -668,11 +674,6 @@ internal bool ShouldEmitAttribute(Symbol target, bool isReturnType, bool emittin
{
Debug.Assert(target is SourceAssemblySymbol || target.ContainingAssembly is SourceAssemblySymbol);

if (HasErrors)
{
throw ExceptionUtilities.Unreachable();
}

// Attribute type is conditionally omitted if both the following are true:
// (a) It has at least one applied/inherited conditional attribute AND
// (b) None of conditional symbols are defined in the source file where the given attribute was defined.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,41 @@ internal override bool HasErrors
}
}

internal override DiagnosticInfo? ErrorInfo
{
get
{
if (HasErrors)
{
switch (AttributeConstructor)
{
case { HasUseSiteError: true } attributeConstructor:
return attributeConstructor.GetUseSiteInfo().DiagnosticInfo;

case { }:
return new CSDiagnosticInfo(ErrorCode.ERR_BogusType, string.Empty);

default:
switch (AttributeClass)
{
case { HasUseSiteError: true } attributeClass:
return attributeClass.GetUseSiteInfo().DiagnosticInfo;

case { } attributeClass:
return new CSDiagnosticInfo(ErrorCode.ERR_MissingPredefinedMember, attributeClass, WellKnownMemberNames.InstanceConstructorName);

default:
return new CSDiagnosticInfo(ErrorCode.ERR_BogusType, string.Empty);
}
}
}
else
{
return null;
}
}
}

internal override bool IsConditionallyOmitted => false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,20 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting
internal sealed class RetargetingAttributeData : CSharpAttributeData
{
private readonly CSharpAttributeData _underlying;
private readonly NamedTypeSymbol _attributeClass;
private readonly NamedTypeSymbol? _attributeClass;
private readonly MethodSymbol? _attributeConstructor;
private readonly ImmutableArray<TypedConstant> _constructorArguments;
private readonly ImmutableArray<KeyValuePair<string, TypedConstant>> _namedArguments;

internal RetargetingAttributeData(
CSharpAttributeData underlying,
NamedTypeSymbol attributeClass,
NamedTypeSymbol? attributeClass,
MethodSymbol? attributeConstructor,
ImmutableArray<TypedConstant> constructorArguments,
ImmutableArray<KeyValuePair<string, TypedConstant>> namedArguments)
{
Debug.Assert(underlying is SourceAttributeData or SynthesizedAttributeData);
Debug.Assert(attributeClass is object || underlying.HasErrors);

_underlying = underlying;
_attributeClass = attributeClass;
Expand All @@ -36,7 +37,7 @@ internal RetargetingAttributeData(
_namedArguments = namedArguments;
}

public override NamedTypeSymbol AttributeClass => _attributeClass;
public override NamedTypeSymbol? AttributeClass => _attributeClass;
public override MethodSymbol? AttributeConstructor => _attributeConstructor;
protected internal override ImmutableArray<TypedConstant> CommonConstructorArguments => _constructorArguments;
protected internal override ImmutableArray<KeyValuePair<string, TypedConstant>> CommonNamedArguments => _namedArguments;
Expand All @@ -46,6 +47,34 @@ internal RetargetingAttributeData(
[MemberNotNullWhen(true, nameof(AttributeClass), nameof(AttributeConstructor))]
internal override bool HasErrors => _underlying.HasErrors || _attributeConstructor is null;

internal override DiagnosticInfo? ErrorInfo
{
get
{
Debug.Assert(AttributeClass is object || _underlying.HasErrors);

if (_underlying.HasErrors)
{
return _underlying.ErrorInfo;
}
else if (HasErrors)
{
Debug.Assert(AttributeConstructor is null);

if (AttributeClass is { HasUseSiteError: true })
{
return AttributeClass.GetUseSiteInfo().DiagnosticInfo;
}

return new CSDiagnosticInfo(ErrorCode.ERR_MissingPredefinedMember, AttributeClass, WellKnownMemberNames.InstanceConstructorName);
}
else
{
return null;
}
}
}

internal override bool IsConditionallyOmitted => _underlying.IsConditionallyOmitted;

internal override Location GetAttributeArgumentLocation(int parameterIndex) => _underlying.GetAttributeArgumentLocation(parameterIndex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ private SourceAttributeData(
bool hasErrors,
bool isConditionallyOmitted)
{
Debug.Assert(compilation is object);
Debug.Assert(applicationNode is object);
Debug.Assert(!isConditionallyOmitted || attributeClass is object && attributeClass.IsConditional);
Debug.Assert(!constructorArguments.IsDefault);
Debug.Assert(!namedArguments.IsDefault);
Expand Down Expand Up @@ -193,6 +195,8 @@ internal override bool HasErrors
}
}

internal override DiagnosticInfo? ErrorInfo => null; // Binder reported errors

protected internal sealed override ImmutableArray<TypedConstant> CommonConstructorArguments
{
get { return _constructorArguments; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1335,6 +1335,9 @@ private WellKnownAttributeData ValidateAttributeUsageAndDecodeWellKnownAttribute
var totalIndex = i + sourceAttributesCount;

CSharpAttributeData attribute = attributesFromNetModules[i];

diagnostics.Add(attribute.ErrorInfo, NoLocation.Singleton);

if (!attribute.HasErrors && ValidateAttributeUsageForNetModuleAttribute(attribute, netModuleNames[i], diagnostics, ref uniqueAttributes))
{
arguments.Attribute = attribute;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ internal FromMethodAndArguments(CSharpCompilation compilation, MethodSymbol well
protected internal override ImmutableArray<TypedConstant> CommonConstructorArguments => _arguments;
protected internal override ImmutableArray<KeyValuePair<string, TypedConstant>> CommonNamedArguments => _namedArguments;
internal override bool HasErrors => false;
internal override DiagnosticInfo? ErrorInfo => null;
internal override bool IsConditionallyOmitted => false;
internal override Location GetAttributeArgumentLocation(int parameterIndex) => NoLocation.Singleton;

Expand Down Expand Up @@ -77,6 +78,7 @@ internal FromSourceAttributeData(SourceAttributeData original)
protected internal override ImmutableArray<TypedConstant> CommonConstructorArguments => _original.CommonConstructorArguments;
protected internal override ImmutableArray<KeyValuePair<string, TypedConstant>> CommonNamedArguments => _original.CommonNamedArguments;
internal override bool HasErrors => _original.HasErrors;
internal override DiagnosticInfo? ErrorInfo => _original.ErrorInfo;
internal override bool IsConditionallyOmitted => _original.IsConditionallyOmitted;

internal override Location GetAttributeArgumentLocation(int parameterIndex) => _original.GetAttributeArgumentLocation(parameterIndex);
Expand Down
Loading

0 comments on commit 06072fb

Please sign in to comment.