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

Follow up on a PROTOTYPE comment in SourceDelegateMethodSymbol.AddDelegateMembers #30079

Open
AlekseyTs opened this issue Sep 20, 2018 · 2 comments

Comments

@AlekseyTs
Copy link
Contributor

        internal static void AddDelegateMembers(
            SourceMemberContainerTypeSymbol delegateType,
            ArrayBuilder<Symbol> symbols,
            DelegateDeclarationSyntax syntax,
            DiagnosticBag diagnostics)
        {
            var compilation = delegateType.DeclaringCompilation;
            Binder binder = delegateType.GetBinder(syntax.ParameterList);
            RefKind refKind;
            TypeSyntax returnTypeSyntax = syntax.ReturnType.SkipRef(out refKind);
            var returnType = binder.BindType(returnTypeSyntax, diagnostics);

            // reuse types to avoid reporting duplicate errors if missing:
            var voidType = TypeSymbolWithAnnotations.Create(binder.GetSpecialType(SpecialType.System_Void, diagnostics, syntax));
            // PROTOTYPE(NullableReferenceTypes): Should the 'object' parameter be considered nullable?
            var objectType = TypeSymbolWithAnnotations.Create(nonNullTypesContext: delegateType, binder.GetSpecialType(SpecialType.System_Object, diagnostics, syntax));
            var intPtrType = TypeSymbolWithAnnotations.Create(binder.GetSpecialType(SpecialType.System_IntPtr, diagnostics, syntax));

            if (returnType.IsRestrictedType(ignoreSpanLikeTypes: true))
            {
                // Method or delegate cannot return type '{0}'
                diagnostics.Add(ErrorCode.ERR_MethodReturnCantBeRefAny, returnTypeSyntax.Location, returnType.TypeSymbol);
            }

            // A delegate has the following members: (see CLI spec 13.6)
            // (1) a method named Invoke with the specified signature
            var invoke = new InvokeMethod(delegateType, refKind, returnType, syntax, binder, diagnostics);
            invoke.CheckDelegateVarianceSafety(diagnostics);
            symbols.Add(invoke);

            // (2) a constructor with argument types (object, System.IntPtr)
            symbols.Add(new Constructor(delegateType, voidType, objectType, intPtrType, syntax));

            if (binder.Compilation.GetSpecialType(SpecialType.System_IAsyncResult).TypeKind != TypeKind.Error &&
                binder.Compilation.GetSpecialType(SpecialType.System_AsyncCallback).TypeKind != TypeKind.Error &&
                // WinRT delegates don't have Begin/EndInvoke methods
                !delegateType.IsCompilationOutputWinMdObj())
            {
                var iAsyncResultType = TypeSymbolWithAnnotations.Create(nonNullTypesContext: delegateType, binder.GetSpecialType(SpecialType.System_IAsyncResult, diagnostics, syntax));
                var asyncCallbackType = TypeSymbolWithAnnotations.Create(nonNullTypesContext: delegateType, binder.GetSpecialType(SpecialType.System_AsyncCallback, diagnostics, syntax));

                // (3) BeginInvoke
                symbols.Add(new BeginInvokeMethod(invoke, iAsyncResultType, objectType, asyncCallbackType, syntax));

                // and (4) EndInvoke methods
                symbols.Add(new EndInvokeMethod(invoke, iAsyncResultType, syntax));
            }

            if (delegateType.DeclaredAccessibility <= Accessibility.Private)
            {
                return;
            }

            HashSet<DiagnosticInfo> useSiteDiagnostics = null;

            if (!delegateType.IsNoMoreVisibleThan(invoke.ReturnType, ref useSiteDiagnostics))
            {
                // Inconsistent accessibility: return type '{1}' is less accessible than delegate '{0}'
                diagnostics.Add(ErrorCode.ERR_BadVisDelegateReturn, delegateType.Locations[0], delegateType, invoke.ReturnType.TypeSymbol);
            }

            foreach (var parameter in invoke.Parameters)
            {
                if (!parameter.Type.IsAtLeastAsVisibleAs(delegateType, ref useSiteDiagnostics))
                {
                    // Inconsistent accessibility: parameter type '{1}' is less accessible than delegate '{0}'
                    diagnostics.Add(ErrorCode.ERR_BadVisDelegateParam, delegateType.Locations[0], delegateType, parameter.Type.TypeSymbol);
                }
            }

            diagnostics.Add(delegateType.Locations[0], useSiteDiagnostics);
        }
@AlekseyTs
Copy link
Contributor Author

I believe the type is used for parameters that are used to refer to an instance that should be used as the receiver for the method invocation. Could be null for static methods.

@jaredpar jaredpar added this to the 16.0 milestone Sep 24, 2018
@jaredpar jaredpar added the Bug label Sep 24, 2018
@jaredpar jaredpar added this to Misc in Nullable Board Jan 28, 2019
@gafter gafter modified the milestones: 16.0, 16.1 Feb 26, 2019
@jcouv jcouv modified the milestones: 16.1, 16.2 Apr 23, 2019
@jcouv jcouv modified the milestones: 16.2, 16.3 Jun 18, 2019
@jcouv jcouv modified the milestones: 16.3, Compiler.Next Jul 11, 2019
@jaredpar
Copy link
Member

Thought about this a bit today and came to the following conclusion:

  1. Constructor should take object?: as you elaborated in the case of a static member this is a valid value.
  2. Invoke / BeginInvoke: in theory we could tweak this to be object? vs. object based on where the declared return type is nullable / non-nullable. I'm not sure that is worth it though. I think we would probably want to use object? because this is only accessible at a reflection level really and there we generally use object?.

@jaredpar jaredpar moved this from Misc to Out Of Scope in Nullable Board Feb 10, 2021
@jaredpar jaredpar modified the milestones: Compiler.Next, Backlog Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Nullable Board
Out Of Scope
Development

No branches or pull requests

4 participants