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

Allow users to define EmbeddedAttribute #76523

Merged
merged 13 commits into from
Jan 3, 2025
22 changes: 22 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,25 @@ struct S
public static void M([UnscopedRef] ref int x) { }
}
```

## `Microsoft.CodeAnalysis.EmbeddedAttribute` is validated on declaration

***Introduced in Visual Studio 2022 version 17.13***

The compiler now validates the shape of `Microsoft.CodeAnalysis.EmbeddedAttribute` when declared in source. Previously, the compiler
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to the PR: it's not clear to me what the Microsoft.CodeAnalysis.EmbeddedAttribute does. Is that documented somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not documented afaik, though we will when we add the SG API. It prevents the compiler from resolving the type outside the current compilation. It's how the compiler itself gets around the "type defined in multiple assemblies" issues when it needs to synthesize an attribute.

would allow user-defined declarations of this attribute, but only when it didn't need to generate one itself. We now validate that:

1. It must be internal
2. It must be a class
333fred marked this conversation as resolved.
Show resolved Hide resolved
3. It must be sealed
4. It must be non-static
5. It must have an internal or public parameterless constructor
6. It must inherit from System.Attribute.
7. It must be allowed on any type declaration (class, struct, interface, enum, or delegate)

```cs
namespace Microsoft.CodeAnalysis;

// Previously, sometimes allowed. Now, CS9271
public class EmbeddedAttribute : Attribute {}
```
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,27 @@ Class C
End Class
```

## `Microsoft.CodeAnalysis.EmbeddedAttribute` is validated on declaration

***Introduced in Visual Studio 2022 version 17.13***

The compiler now validates the shape of `Microsoft.CodeAnalysis.EmbeddedAttribute` when declared in source. Previously, the compiler
would allow user-defined declarations of this attribute with any shape. We now validate that:

1. It must be Friend
2. It must be a Class
3. It must be NotInheritable
4. It must not be a Module
5. It must have a Public parameterless constructor
6. It must inherit from System.Attribute.
7. It must be allowed on any type declaration (Class, Structure, Interface, Enum, or Delegate)

```vb
Namespace Microsoft.CodeAnalysis

' Previously allowed. Now, BC37335
Public Class EmbeddedAttribute
Inherits Attribute
End Class
End Namespace
```
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5908,6 +5908,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_TypeReserved" xml:space="preserve">
<value>The type name '{0}' is reserved to be used by the compiler.</value>
</data>
<data name="ERR_EmbeddedAttributeMustFollowPattern" xml:space="preserve">
<value>The type 'Microsoft.CodeAnalysis.EmbeddedAttribute' must be non-generic, internal, sealed, non-static, have a parameterless constructor, inherit from System.Attribute, and be able to be applied to any type.</value>
</data>
<data name="ERR_InExtensionMustBeValueType" xml:space="preserve">
<value>The first 'in' or 'ref readonly' parameter of the extension method '{0}' must be a concrete (non-generic) value type.</value>
</data>
Expand Down
54 changes: 35 additions & 19 deletions src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.Emit;
using Microsoft.CodeAnalysis.PooledObjects;
Expand All @@ -32,7 +33,7 @@ internal abstract class PEAssemblyBuilderBase : PEModuleBuilder, Cci.IAssemblyRe
/// <summary>This is a cache of a subset of <seealso cref="_lazyFiles"/>. We don't include manifest resources in ref assemblies</summary>
private ImmutableArray<Cci.IFileReference> _lazyFilesWithoutManifestResources;

private SynthesizedEmbeddedAttributeSymbol _lazyEmbeddedAttribute;
private NamedTypeSymbol _lazyEmbeddedAttribute;
private SynthesizedEmbeddedAttributeSymbol _lazyIsReadOnlyAttribute;
private SynthesizedEmbeddedAttributeSymbol _lazyRequiresLocationAttribute;
private SynthesizedEmbeddedAttributeSymbol _lazyParamCollectionAttribute;
Expand Down Expand Up @@ -94,7 +95,11 @@ internal sealed override ImmutableArray<NamedTypeSymbol> GetEmbeddedTypes(Bindin

CreateEmbeddedAttributesIfNeeded(diagnostics);

builder.AddIfNotNull(_lazyEmbeddedAttribute);
if (_lazyEmbeddedAttribute is SynthesizedEmbeddedAttributeSymbol)
{
builder.Add(_lazyEmbeddedAttribute);
}

builder.AddIfNotNull(_lazyIsReadOnlyAttribute);
builder.AddIfNotNull(_lazyRequiresLocationAttribute);
builder.AddIfNotNull(_lazyParamCollectionAttribute);
Expand Down Expand Up @@ -387,7 +392,8 @@ private void CreateEmbeddedAttributesIfNeeded(BindingDiagnosticBag diagnostics)
ref _lazyEmbeddedAttribute,
diagnostics,
AttributeDescription.CodeAnalysisEmbeddedAttribute,
createParameterlessEmbeddedAttributeSymbol);
createParameterlessEmbeddedAttributeSymbol,
allowUserDefinedAttribute: true);

if ((needsAttributes & EmbeddableAttributes.IsReadOnlyAttribute) != 0)
{
Expand Down Expand Up @@ -544,16 +550,32 @@ private SynthesizedEmbeddedRefSafetyRulesAttributeSymbol CreateRefSafetyRulesAtt
GetWellKnownType(WellKnownType.System_Attribute, diagnostics),
GetSpecialType(SpecialType.System_Int32, diagnostics));

#nullable enable
private void CreateAttributeIfNeeded<T>(
ref T symbol,
BindingDiagnosticBag diagnostics,
AttributeDescription description,
Func<string, NamespaceSymbol, BindingDiagnosticBag, T> factory)
where T : SynthesizedEmbeddedAttributeSymbolBase
Func<string, NamespaceSymbol, BindingDiagnosticBag, T> factory,
bool allowUserDefinedAttribute = false)
where T : NamedTypeSymbol
{
Debug.Assert(!allowUserDefinedAttribute || typeof(T) == typeof(NamedTypeSymbol));
if (symbol is null)
{
AddDiagnosticsForExistingAttribute(description, diagnostics);
var userDefinedAttribute = getExistingType(description);

if (userDefinedAttribute is not null)
{
if (allowUserDefinedAttribute)
{
symbol = (T)userDefinedAttribute;
return;
}
else
{
diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.GetFirstLocation(), description.FullName);
}
}

var containingNamespace = GetOrSynthesizeNamespace(description.Namespace);

Expand All @@ -567,23 +589,17 @@ private void CreateAttributeIfNeeded<T>(

AddSynthesizedDefinition(containingNamespace, symbol);
}
}

#nullable enable

private void AddDiagnosticsForExistingAttribute(AttributeDescription description, BindingDiagnosticBag diagnostics)
{
var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
Debug.Assert(userDefinedAttribute is null || (object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);
Debug.Assert(userDefinedAttribute?.IsErrorType() != true);

if (userDefinedAttribute is not null)
NamedTypeSymbol? getExistingType(AttributeDescription description)
{
diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.GetFirstLocation(), description.FullName);
var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
Debug.Assert(userDefinedAttribute is null || (object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);
Debug.Assert(userDefinedAttribute?.IsErrorType() != true);

return userDefinedAttribute;
}
}

#nullable disable

protected NamespaceSymbol GetOrSynthesizeNamespace(string namespaceFullName)
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2356,6 +2356,8 @@ internal enum ErrorCode
WRN_UnscopedRefAttributeOldRules = 9269,
WRN_InterceptsLocationAttributeUnsupportedSignature = 9270,

ERR_EmbeddedAttributeMustFollowPattern = 9271,

// Note: you will need to do the following after adding errors:
// 1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1844,6 +1844,7 @@ or ErrorCode.ERR_RefReturnReadonlyNotField
or ErrorCode.ERR_RefReturnReadonlyNotField2
or ErrorCode.ERR_ExplicitReservedAttr
or ErrorCode.ERR_TypeReserved
or ErrorCode.ERR_EmbeddedAttributeMustFollowPattern
or ErrorCode.ERR_RefExtensionMustBeValueTypeOrConstrainedToOne
or ErrorCode.ERR_InExtensionMustBeValueType
or ErrorCode.ERR_FieldsInRoStruct
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#nullable disable

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
Expand Down Expand Up @@ -1414,7 +1415,9 @@ internal override bool HasCodeAnalysisEmbeddedAttribute
get
{
var data = GetEarlyDecodedWellKnownAttributeData();
333fred marked this conversation as resolved.
Show resolved Hide resolved
return data != null && data.HasCodeAnalysisEmbeddedAttribute;
return (data != null && data.HasCodeAnalysisEmbeddedAttribute)
// If this is Microsoft.CodeAnalysis.EmbeddedAttribute, we'll synthesize EmbeddedAttribute even if it's not applied.
|| this.IsMicrosoftCodeAnalysisEmbeddedAttribute();
}
}

Expand Down Expand Up @@ -1748,6 +1751,21 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
ImmutableArray.Create(new TypedConstant(compilation.GetWellKnownType(WellKnownType.System_Type), TypedConstantKind.Type, originalType)),
isOptionalUse: true));
}

if (this.IsMicrosoftCodeAnalysisEmbeddedAttribute() && GetEarlyDecodedWellKnownAttributeData() is null or { HasCodeAnalysisEmbeddedAttribute: false })
333fred marked this conversation as resolved.
Show resolved Hide resolved
{
// This is Microsoft.CodeAnalysis.EmbeddedAttribute, and the user didn't manually apply this attribute to itself. Grab the parameterless constructor
333fred marked this conversation as resolved.
Show resolved Hide resolved
// and apply it; if there isn't a parameterless constructor, there would have been a declaration diagnostic

var parameterlessConstructor = InstanceConstructors.FirstOrDefault(c => c.ParameterCount == 0);

if (parameterlessConstructor is not null)
{
AddSynthesizedAttribute(
ref attributes,
SynthesizedAttributeData.Create(DeclaringCompilation, parameterlessConstructor, arguments: [], namedArguments: []));
}
}
}

#endregion
Expand Down Expand Up @@ -1905,6 +1923,34 @@ protected override void AfterMembersCompletedChecks(BindingDiagnosticBag diagnos
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportInlineArrayTypes, GetFirstLocation());
}
}

if (this.IsMicrosoftCodeAnalysisEmbeddedAttribute())
{
// This is a user-defined implementation of the special attribute Microsoft.CodeAnalysis.EmbeddedAttribute. It needs to follow specific rules:
// 1. It must be internal
// 2. It must be a class
// 3. It must be sealed
// 4. It must be non-static
// 5. It must have an internal or public parameterless constructor
// 6. It must inherit from System.Attribute
// 7. It must be allowed on any type declaration (class, struct, interface, enum, or delegate)
// 8. It must be non-generic (checked as part of IsMicrosoftCodeAnalysisEmbeddedAttribute, we don't error on this because both types can exist)

const AttributeTargets expectedTargets = AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface | AttributeTargets.Enum | AttributeTargets.Delegate;

if (DeclaredAccessibility != Accessibility.Internal
|| TypeKind != TypeKind.Class
|| !IsSealed
|| IsStatic
|| !InstanceConstructors.Any(c => c is { ParameterCount: 0, DeclaredAccessibility: Accessibility.Internal or Accessibility.Public })
|| !this.DeclaringCompilation.IsAttributeType(this)
|| (GetAttributeUsageInfo().ValidTargets & expectedTargets) != expectedTargets)
{
// The type 'Microsoft.CodeAnalysis.EmbeddedAttribute' must be non-generic, internal, sealed, non-static, have a parameterless constructor, inherit from System.Attribute, and be able to be applied to any type.
diagnostics.Add(ErrorCode.ERR_EmbeddedAttributeMustFollowPattern, GetFirstLocation());
}

}
}
}
}
11 changes: 11 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2137,6 +2137,17 @@ internal static bool IsWellKnownTypeLock(this TypeSymbol typeSymbol)
typeSymbol.IsContainedInNamespace(nameof(System), nameof(System.Threading));
}

internal static bool IsMicrosoftCodeAnalysisEmbeddedAttribute(this TypeSymbol typeSymbol)
{
return typeSymbol is NamedTypeSymbol
{
Name: "EmbeddedAttribute",
Arity: 0,
ContainingType: null,
}
&& typeSymbol.IsContainedInNamespace("Microsoft", "CodeAnalysis");
}

private static bool IsWellKnownInteropServicesTopLevelType(this TypeSymbol typeSymbol, string name)
{
if (typeSymbol.Name != name || typeSymbol.ContainingType is object)
Expand Down
7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading