Merge pull request #41847 from dotnet/features/local-function-attributes

Features/local function attributes

Author: RikkiGibson

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -39,6 +39,23 @@ internal virtual ImmutableHashSet<Symbol> LockedOrDisposedVariables
         /// </remarks>
         public virtual BoundStatement BindStatement(StatementSyntax node, DiagnosticBag diagnostics)
         {
+            if (node.AttributeLists.Count > 0)
+            {
+                var attributeList = node.AttributeLists[0];
+
+                // Currently, attributes are only allowed on local-functions.
+                if (node.Kind() == SyntaxKind.LocalFunctionStatement)
+                {
+                    CheckFeatureAvailability(attributeList, MessageID.IDS_FeatureLocalFunctionAttributes, diagnostics);
+                }
+                else if (node.Kind() != SyntaxKind.Block)
+                {
+                    // Don't explicitly error here for blocks.  Some codepaths bypass BindStatement
+                    // to directly call BindBlock.
+                    Error(diagnostics, ErrorCode.ERR_AttributesNotAllowed, attributeList);
+                }
+            }
+
             Debug.Assert(node != null);
             BoundStatement result;
             switch (node.Kind())
@@ -554,13 +571,19 @@ private BoundStatement BindLocalFunctionStatement(LocalFunctionStatementSyntax n
             {
                 expressionBody = runAnalysis(BindExpressionBodyAsBlock(node.ExpressionBody, diagnostics), diagnostics);
             }
-            else
+            else if (!hasErrors && (!localSymbol.IsExtern || !localSymbol.IsStatic))
             {
                 hasErrors = true;
                 diagnostics.Add(ErrorCode.ERR_LocalFunctionMissingBody, localSymbol.Locations[0], localSymbol);
             }
 
-            Debug.Assert(blockBody != null || expressionBody != null || hasErrors);
+            if (!hasErrors && (blockBody != null || expressionBody != null) && localSymbol.IsExtern)
+            {
+                hasErrors = true;
+                diagnostics.Add(ErrorCode.ERR_ExternHasBody, localSymbol.Locations[0], localSymbol);
+            }
+
+            Debug.Assert(blockBody != null || expressionBody != null || (localSymbol.IsExtern && localSymbol.IsStatic) || hasErrors);
 
             localSymbol.GetDeclarationDiagnostics(diagnostics);
 
@@ -1673,6 +1696,11 @@ internal virtual BoundBlock BindEmbeddedBlock(BlockSyntax node, DiagnosticBag di
 
         private BoundBlock BindBlock(BlockSyntax node, DiagnosticBag diagnostics)
         {
+            if (node.AttributeLists.Count > 0)
+            {
+                Error(diagnostics, ErrorCode.ERR_AttributesNotAllowed, node.AttributeLists[0]);
+            }
+
             var binder = GetBinder(node);
             Debug.Assert(binder != null);
 

src/Compilers/CSharp/Portable/BoundTree/BoundLocalFunctionStatement.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+#nullable enable
+
 using System;
 using System.Collections.Generic;
 using System.Text;
@@ -10,6 +12,6 @@ namespace Microsoft.CodeAnalysis.CSharp
 {
     internal partial class BoundLocalFunctionStatement
     {
-        public BoundBlock Body { get => BlockBody ?? ExpressionBody; }
+        public BoundBlock? Body { get => BlockBody ?? ExpressionBody; }
     }
 }

src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs

@@ -611,7 +611,7 @@ private BoundLambda ReallyBind(NamedTypeSymbol delegateType)
             if (IsAsync)
             {
                 Debug.Assert(lambdaSymbol.IsAsync);
-                SourceOrdinaryMethodSymbol.ReportAsyncParameterErrors(lambdaSymbol.Parameters, diagnostics, lambdaSymbol.DiagnosticLocation);
+                lambdaSymbol.ReportAsyncParameterErrors(diagnostics, lambdaSymbol.DiagnosticLocation);
             }
 
             var result = new BoundLambda(_unboundLambda.Syntax, _unboundLambda, block, diagnostics.ToReadOnlyAndFree(), lambdaBodyBinder, delegateType, inferredReturnType: default)

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -126,6 +126,12 @@
   <data name="IDS_FeatureSwitchExpression" xml:space="preserve">
     <value>&lt;switch expression&gt;</value>
   </data>
+  <data name="IDS_FeatureLocalFunctionAttributes" xml:space="preserve">
+    <value>local function attributes</value>
+  </data>
+  <data name="IDS_FeatureExternLocalFunctions" xml:space="preserve">
+    <value>extern local functions</value>
+  </data>
   <data name="IDS_RELATEDERROR" xml:space="preserve">
     <value>(Location of symbol related to previous error)</value>
   </data>
@@ -1023,6 +1029,9 @@
   <data name="ERR_ConditionalOnOverride" xml:space="preserve">
     <value>The Conditional attribute is not valid on '{0}' because it is an override method</value>
   </data>
+  <data name="ERR_ConditionalOnLocalFunction" xml:space="preserve">
+    <value>Local function '{0}' must be 'static' in order to use the Conditional attribute</value>
+  </data>
   <data name="ERR_PointerInAsOrIs" xml:space="preserve">
     <value>Neither 'is' nor 'as' is valid on pointer types</value>
   </data>
@@ -1713,7 +1722,7 @@ If such a class is used as a base class and if the deriving class defines a dest
     <value>Cannot update '{0}'; attribute '{1}' is missing.</value>
   </data>
   <data name="ERR_DllImportOnGenericMethod" xml:space="preserve">
-    <value>The DllImport attribute cannot be applied to a method that is generic or contained in a generic type.</value>
+    <value>The DllImport attribute cannot be applied to a method that is generic or contained in a generic method or type.</value>
   </data>
   <data name="ERR_FieldCantBeRefAny" xml:space="preserve">
     <value>Field or property cannot be of type '{0}'</value>
@@ -5570,7 +5579,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>Local function is declared but never used</value>
   </data>
   <data name="ERR_LocalFunctionMissingBody" xml:space="preserve">
-    <value>'{0}' is a local function and must therefore always have a body.</value>
+    <value>Local function '{0}' must declare a body because it is not marked 'static extern'.</value>
   </data>
   <data name="ERR_InvalidDebugInfo" xml:space="preserve">
     <value>Unable to read debug information of method '{0}' (token 0x{1:X8}) from assembly '{2}'</value>

src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

@@ -1638,8 +1638,7 @@ private static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationSta
 
             BoundBlock body;
 
-            var sourceMethod = method as SourceMemberMethodSymbol;
-            if ((object)sourceMethod != null)
+            if (method is SourceMemberMethodSymbol sourceMethod)
             {
                 CSharpSyntaxNode syntaxNode = sourceMethod.SyntaxNode;
                 var constructorSyntax = syntaxNode as ConstructorDeclarationSyntax;
@@ -1656,20 +1655,8 @@ private static BoundBlock BindMethodBody(MethodSymbol method, TypeCompilationSta
 
                 ExecutableCodeBinder bodyBinder = sourceMethod.TryGetBodyBinder();
 
-                if (sourceMethod.IsExtern)
+                if (sourceMethod.IsExtern || sourceMethod.IsDefaultValueTypeConstructor())
                 {
-                    if (bodyBinder == null)
-                    {
-                        // Generate warnings only if we are not generating ERR_ExternHasBody or ERR_ExternHasConstructorInitializer errors
-                        GenerateExternalMethodWarnings(sourceMethod, diagnostics);
-                    }
-
-                    return null;
-                }
-
-                if (sourceMethod.IsDefaultValueTypeConstructor())
-                {
-                    // No body for default struct constructor.
                     return null;
                 }
 
@@ -1958,18 +1945,6 @@ internal static BoundCall GenerateBaseParameterlessConstructorInitializer(Method
             { WasCompilerGenerated = true };
         }
 
-        private static void GenerateExternalMethodWarnings(SourceMemberMethodSymbol methodSymbol, DiagnosticBag diagnostics)
-        {
-            if (methodSymbol.GetAttributes().IsEmpty && !methodSymbol.ContainingType.IsComImport)
-            {
-                // external method with no attributes
-                var errorCode = (methodSymbol.MethodKind == MethodKind.Constructor || methodSymbol.MethodKind == MethodKind.StaticConstructor) ?
-                    ErrorCode.WRN_ExternCtorNoImplementation :
-                    ErrorCode.WRN_ExternMethodNoImplementation;
-                diagnostics.Add(errorCode, methodSymbol.Locations[0], methodSymbol);
-            }
-        }
-
         /// <summary>
         /// Returns true if the method is a constructor and has a this() constructor initializer.
         /// </summary>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1759,6 +1759,8 @@ internal enum ErrorCode
 
         ERR_StdInOptionProvidedButConsoleInputIsNotRedirected = 8782,
 
+        ERR_ConditionalOnLocalFunction = 8783,
+
         // Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
     }
 }

src/Compilers/CSharp/Portable/Errors/MessageID.cs

@@ -187,6 +187,8 @@ internal enum MessageID
         IDS_FeatureSwitchExpression = MessageBase + 12763,
         IDS_FeatureAsyncUsing = MessageBase + 12764,
         IDS_FeatureLambdaDiscardParameters = MessageBase + 12765,
+        IDS_FeatureLocalFunctionAttributes = MessageBase + 12766,
+        IDS_FeatureExternLocalFunctions = MessageBase + 12767,
     }
 
     // Message IDs may refer to strings that need to be localized.
@@ -293,8 +295,10 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
             // Checks are in the LanguageParser unless otherwise noted.
             switch (feature)
             {
-                // Preview features.
+                // C# preview features.
                 case MessageID.IDS_FeatureLambdaDiscardParameters: // semantic check
+                case MessageID.IDS_FeatureLocalFunctionAttributes: // syntax check
+                case MessageID.IDS_FeatureExternLocalFunctions: // syntax check
                     return LanguageVersion.Preview;
 
                 // C# 8.0 features.

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -1919,6 +1919,14 @@ protected override void VisitLocalFunctionUse(
             SyntaxNode syntax,
             bool isCall)
         {
+            // Do not use this overload in NullableWalker. Use the overload below instead.
+            throw ExceptionUtilities.Unreachable;
+        }
+
+        private void VisitLocalFunctionUse(LocalFunctionSymbol symbol)
+        {
+            Debug.Assert(!IsConditionalState);
+            var localFunctionState = GetOrCreateLocalFuncUsages(symbol);
             if (Join(ref localFunctionState.StartingState, ref State) &&
                 localFunctionState.Visited)
             {
@@ -3644,10 +3652,6 @@ public override BoundNode VisitCall(BoundCall node)
             // Note: we analyze even omitted calls
             TypeWithState receiverType = VisitCallReceiver(node);
             ReinferMethodAndVisitArguments(node, receiverType);
-            if (node.Method?.OriginalDefinition is LocalFunctionSymbol localFunc)
-            {
-                VisitLocalFunctionUse(localFunc, node.Syntax, isCall: true);
-            }
             return null;
         }
 
@@ -4084,6 +4088,11 @@ private ImmutableArray<VisitArgumentResult> VisitArguments(
                 }
             }
 
+            if (node is BoundCall { Method: { OriginalDefinition: LocalFunctionSymbol localFunction } })
+            {
+                VisitLocalFunctionUse(localFunction);
+            }
+
             if (!node.HasErrors && !parametersOpt.IsDefault)
             {
                 // Visit outbound assignments and post-conditions
@@ -5624,7 +5633,7 @@ private TypeWithState VisitConversion(
                         {
                             if (method?.OriginalDefinition is LocalFunctionSymbol localFunc)
                             {
-                                VisitLocalFunctionUse(localFunc, group.Syntax, isCall: false);
+                                VisitLocalFunctionUse(localFunc);
                             }
                             method = CheckMethodGroupReceiverNullability(group, delegateType, method, conversion.IsExtensionMethod);
                         }
@@ -6177,7 +6186,7 @@ public override BoundNode VisitDelegateCreationExpression(BoundDelegateCreationE
 
             if (node.MethodOpt?.OriginalDefinition is LocalFunctionSymbol localFunc)
             {
-                VisitLocalFunctionUse(localFunc, node.Syntax, isCall: true);
+                VisitLocalFunctionUse(localFunc);
             }
 
             var delegateType = (NamedTypeSymbol)node.Type;