Local function GetAttributes #39135

RikkiGibson · 2019-10-08T18:28:42Z

Related to #38801

LangVersion errors on all local function attributes (incl. parameters and type parameters)
GetAttributes()
Attribute binding, rejecting attributes on invalid targets
~~- [ ] tests for speculative semantic model~~ moving to test plan

PTAL @dotnet/roslyn-compiler @cston

…utes' into features/local-function-attributes

cston · 2019-10-09T16:11:49Z

src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs

+        private void ReportAttributesDisallowed(SyntaxList<AttributeListSyntax> attributes, DiagnosticBag diagnostics)
        {
-            foreach (var attrList in attributes)
+            var diagnosticInfo = MessageID.IDS_FeatureLocalFunctionAttributes.GetFeatureAvailabilityDiagnosticInfoOpt(ScopeBinder.Compilation);


ScopeBinder.Compilation [](start = 118, length = 23)

Consider using DeclaringCompilation instead for consistency with other uses. #Resolved

So, this needs a look, because DeclaringCompilation is null for some reason in EE scenarios. Perhaps that is a bug? #Resolved

filed #39165 to follow up on this. #Resolved

cston · 2019-10-09T16:20:05Z

src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs

+                return lazyCustomAttributesBag.Attributes;
+            }
+
+            var bagCreatedOnThisThread = LoadAndValidateAttributes(OneOrMany.Create(_syntax.AttributeLists), ref _lazyCustomAttributesBag);


var bagCreatedOnThisThread = [](start = 12, length = 28)

Consider using _ = or ignore the return value completely.

To the comment below, there shouldn't be anything else to do for the thread that succeeds.

Same comments for GetReturnTypeAttributes. #Resolved

cston · 2019-10-09T16:23:22Z

src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs

+        public override ImmutableArray<CSharpAttributeData> GetAttributes()
+        {
+            var lazyCustomAttributesBag = _lazyCustomAttributesBag;
+            if (lazyCustomAttributesBag != null)


!= null [](start = 40, length = 7)

Consider using == null and moving LoadAndValidateAttributes inside the if, so there's only one return statement. Same for GetReturnTypeAttributes. #Resolved

cston · 2019-10-09T16:29:20Z

src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs

-            //Assert.Single(attributes);
+            Assert.Single(attributes);
+
+            var attributeData = attributes[0];


Minor point: these could be combined in each of these tests.
var attributeData = attributes.Single(); #Resolved

cston · 2019-10-09T16:58:29Z

src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs

-            var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular);
-            compilation.GetDiagnostics().Where(d => d.Code != (int)ErrorCode.ERR_AttributesInLocalFuncDecl).Verify(
+            var compilation = CreateCompilationWithMscorlib45(source, options: TestOptions.DebugExe, parseOptions: TestOptions.RegularPreview);
+            compilation.GetDiagnostics().Verify(


GetDiagnostics().Verify [](start = 24, length = 23)

VerifyDiagnostics #Resolved

RikkiGibson · 2019-10-09T20:59:27Z

Could I get a second review please? @dotnet/roslyn-compiler

…utes' into features/local-function-attributes

agocke · 2019-10-10T08:17:30Z

I'm not seeing a test that confirms this is blocked in an older language version. Am I missing it? #Resolved

RikkiGibson · 2019-10-10T17:07:10Z

The GitHub diff seems to be hiding it, but it's here:

roslyn/src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs

Line 588 in 3c3f4ff

public void LocalFunctionAttribute_LangVersionError()

#Resolved

RikkiGibson · 2019-10-14T18:36:39Z

Could I get a second review please @dotnet/roslyn-compiler

src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs

chsienki

RikkiGibson added 6 commits September 30, 2019 15:43

Report LangVersion errors on attributes in local function parameters

2b968fb

Check LangVersion in symbol constructor

a9b2dd2

Merge remote-tracking branch 'upstream/features/local-function-attrib…

0371dd1

…utes' into features/local-function-attributes

Implement GetAttributes() on LocalFunctionSymbol

8ffd7fb

Add tests

b5e49d7

Add SemanticModel test using AttributeSyntax

d488e8c

RikkiGibson added the Area-Compilers label Oct 8, 2019

RikkiGibson added this to the Compiler.Next milestone Oct 8, 2019

RikkiGibson requested a review from a team as a code owner October 8, 2019 18:28

RikkiGibson added 2 commits October 8, 2019 14:43

Fix a few failing tests

774e546

Just use the ScopeBinder, we only need parse options

7db1453

cston reviewed Oct 9, 2019

View reviewed changes

cston approved these changes Oct 9, 2019

View reviewed changes

RikkiGibson added 2 commits October 9, 2019 13:05

Address feedback

8384998

Get parse options through syntax, not through compilation

58f6b85

RikkiGibson requested a review from a team October 9, 2019 20:59

RikkiGibson requested review from a team and srivatsn as code owners October 9, 2019 21:32

RikkiGibson added 2 commits October 9, 2019 15:17

Merge remote-tracking branch 'upstream/features/local-function-attrib…

c350e14

…utes' into features/local-function-attributes

Fix nullable warning produced from recent changes in master

a0427da

RikkiGibson force-pushed the features/local-function-attributes branch from 8e62aab to a0427da Compare October 9, 2019 22:20

Fix new test from base branch

3c3f4ff

RikkiGibson removed request for a team and srivatsn October 10, 2019 21:29

Fix whitespace

9220335

RikkiGibson mentioned this pull request Oct 10, 2019

Local function attributes emit #39226

Merged

chsienki reviewed Oct 14, 2019

View reviewed changes

src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs Outdated Show resolved Hide resolved

chsienki approved these changes Oct 14, 2019

View reviewed changes

RikkiGibson added 2 commits October 14, 2019 15:11

Reduce diagnostic noise

6fb6b56

Fix

1faf4ba

RikkiGibson merged commit df8d805 into dotnet:features/local-function-attributes Oct 15, 2019

Local function GetAttributes #39135

Local function GetAttributes #39135

Uh oh!

Conversation

RikkiGibson commented Oct 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cston Oct 9, 2019 • edited by RikkiGibson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston Oct 9, 2019 • edited by RikkiGibson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston Oct 9, 2019 • edited by RikkiGibson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston Oct 9, 2019 • edited by RikkiGibson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston Oct 9, 2019 • edited by RikkiGibson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson commented Oct 9, 2019

Uh oh!

agocke commented Oct 10, 2019 • edited by RikkiGibson Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson commented Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson commented Oct 14, 2019

Uh oh!

Uh oh!

chsienki left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RikkiGibson commented Oct 8, 2019 •

edited

Loading

cston Oct 9, 2019 •

edited by RikkiGibson

Loading

RikkiGibson Oct 9, 2019 •

edited

Loading

RikkiGibson Oct 9, 2019 •

edited

Loading

cston Oct 9, 2019 •

edited by RikkiGibson

Loading

cston Oct 9, 2019 •

edited by RikkiGibson

Loading

cston Oct 9, 2019 •

edited by RikkiGibson

Loading

cston Oct 9, 2019 •

edited by RikkiGibson

Loading

agocke commented Oct 10, 2019 •

edited by RikkiGibson

Loading

RikkiGibson commented Oct 10, 2019 •

edited

Loading