Extensions: fix crash with targeted attribute on extension block#82463
Open
jcouv wants to merge 3 commits intodotnet:mainfrom
Open
Extensions: fix crash with targeted attribute on extension block#82463jcouv wants to merge 3 commits intodotnet:mainfrom
jcouv wants to merge 3 commits intodotnet:mainfrom
Conversation
jjonescz
reviewed
Feb 20, 2026
|
|
||
| case AttributeLocation.None: | ||
| // No attributes are allowed on this declaration (e.g. extension blocks). | ||
| diagnostics.Add(ErrorCode.ERR_AttributeNotAllowedOnExtensionBlock, targetOpt.Identifier.GetLocation()); |
Member
There was a problem hiding this comment.
Looks like the diagnostic is extension-specific. Should we update the comment (e.g. doesn't seem correct) and assert that the symbol is an extension? #Resolved
| var comp = CreateCompilation(src); | ||
| comp.VerifyEmitDiagnostics( | ||
| // (3,6): error CS0592: Attribute 'System.Obsolete' is not valid on this declaration type. It is only valid on 'class, struct, enum, constructor, method, property, indexer, field, event, interface, delegate' declarations. | ||
| // (3,6): error CS9360: Attributes are not allowed on extension blocks. |
AlekseyTs
reviewed
Feb 20, 2026
| AttributeLocation IAttributeTargetSymbol.DefaultAttributeLocation | ||
| { | ||
| get { return AttributeLocation.Type; } | ||
| get { return IsExtension ? AttributeLocation.None : AttributeLocation.Type; } |
Contributor
AlekseyTs
reviewed
Feb 20, 2026
| var model = comp.GetSemanticModel(tree); | ||
| var type = tree.GetRoot().DescendantNodes().OfType<ExtensionBlockDeclarationSyntax>().Single(); | ||
| var symbol = model.GetDeclaredSymbol(type); | ||
| AssertEx.SetEqual([], symbol.GetAttributes().Select(a => a.ToString())); |
Contributor
AlekseyTs
reviewed
Feb 20, 2026
| var model = comp.GetSemanticModel(tree); | ||
| var type = tree.GetRoot().DescendantNodes().OfType<ExtensionBlockDeclarationSyntax>().Single(); | ||
| var symbol = model.GetDeclaredSymbol(type); | ||
| AssertEx.SetEqual([], symbol.GetAttributes().Select(a => a.ToString())); |
Contributor
AlekseyTs
reviewed
Feb 20, 2026
| var model = comp.GetSemanticModel(tree); | ||
| var type = tree.GetRoot().DescendantNodes().OfType<ExtensionBlockDeclarationSyntax>().Single(); | ||
| var symbol = model.GetDeclaredSymbol(type); | ||
| AssertEx.SetEqual([], symbol.TypeParameters[0].GetAttributes().Select(a => a.ToString())); |
Contributor
AlekseyTs
reviewed
Feb 20, 2026
| var model = comp.GetSemanticModel(tree); | ||
| var type = tree.GetRoot().DescendantNodes().OfType<ExtensionBlockDeclarationSyntax>().Single(); | ||
| var symbol = model.GetDeclaredSymbol(type); | ||
| AssertEx.SetEqual([], symbol.GetAttributes().Select(a => a.ToString())); |
Contributor
AlekseyTs
reviewed
Feb 20, 2026
| comp.VerifyEmitDiagnostics( | ||
| // (3,6): warning CS0658: 'fake' is not a recognized attribute location. Valid attribute locations for this declaration are ''. All attributes in this block will be ignored. | ||
| // [fake: My] | ||
| Diagnostic(ErrorCode.WRN_InvalidAttributeLocation, "fake").WithArguments("fake", "").WithLocation(3, 6)); |
Contributor
Contributor
|
Done with review pass (commit 1) |
added 2 commits
February 20, 2026 22:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #82445
Fixes #82444
Relates to feature #76130
Problem: Placing an attribute with a target specifier (such as
[return: Obsolete]) on an extension block caused a crash inMatchAttributeTarget. TheallowedTargets == Nonebranch assumed only interactive/global symbols (Assembly/Module) could have no allowed targets, and threw on any otherDefaultAttributeLocationvalue.Extension blocks (which are types with
AllowedAttributeLocations == None) hit the throwing branch.Fix:
DefaultAttributeLocation = Nonefor extension types inSourceNamedTypeSymbolNonecase inMatchAttributeTarget[MyAttribute] extension(int i)and the new check[return: MyAttribute] extension(int i)We should discuss whether one or both of these scenarios should use a warning instead of an error.