Extensions: allow extern and disallow in nameof #79021
Conversation
jcouv
added
Area-Compilers
Feature - Extension Everything
| <value>An extension member syntax is disallowed in nested position within an extension member syntax</value> | ||
| </data> | ||
| <data name="ERR_NameofExtensionMember" xml:space="preserve"> | ||
| <value>Extension member are not allowed as an argument to 'nameof'.</value> |
There was a problem hiding this comment.
"members" (plural) or "are is allowed"? #Resolved
| { | ||
| CheckDefinitionInvariant(); | ||
| return AdaptedMethodSymbol.GetDllImportData(); | ||
| return AdaptedMethodSymbol.ContainingType.IsExtension ? null : AdaptedMethodSymbol.GetDllImportData(); |
There was a problem hiding this comment.
Do we need to adjust GetImplementationAttributes similarly? #Resolved
| { | ||
| extension(int) | ||
| { | ||
| [DllImport("something.dll")] |
There was a problem hiding this comment.
Should we also test [MethodImpl(MethodImplOptions.InternalCall)]? Something like here:
There was a problem hiding this comment.
Yup. Good catch and thanks for the pointer
| // (4,12): error CS1061: '<anonymous type: string Name, int Age>' does not contain a definition for 'P' and no accessible extension method 'P' accepting a first argument of type '<anonymous type: string Name, int Age>' could be found (are you missing a using directive or an assembly reference?) | ||
| // _ = person.P; | ||
| Diagnostic(ErrorCode.ERR_NoSuchMemberOrExtension, "P").WithArguments("<anonymous type: string Name, int Age>", "P").WithLocation(4, 12)); | ||
| CompileAndVerify(comp, expectedOutput: "method method2 property").VerifyDiagnostics(); |
There was a problem hiding this comment.
What change caused this test to start working? #Resolved
There was a problem hiding this comment.
What change caused this test to start working?
There was a bug in the test scenario
| internal sealed override bool IsExternal => false; | ||
| public sealed override bool IsExtern => _originalMethod.IsExtern; | ||
| public sealed override DllImportData? GetDllImportData() => _originalMethod.GetDllImportData(); | ||
| internal sealed override bool IsExternal => _originalMethod.IsExternal; |
There was a problem hiding this comment.
Should we just delete these since the base class WrappedMethodSymbol already has this implementation? #Resolved
There was a problem hiding this comment.
We've been leaning toward abstract+sealed design (pushing implementations down into leaves) so I intentionally left those despite the base/virtual implementation being sufficient
| } | ||
| """; | ||
| var comp = CreateCompilation(source); | ||
| comp.VerifyEmitDiagnostics( |
| } | ||
|
|
||
| [Fact] | ||
| public void Extern_02() |
| } | ||
|
|
||
| [Fact] | ||
| public void Extern_04() |
|
Done with review pass (commit 3) #Closed |
| // extern void M(); | ||
| Diagnostic(ErrorCode.WRN_ExternMethodNoImplementation, "M").WithArguments("C.M()").WithLocation(3, 17)); | ||
|
|
||
| // Error: Method marked Abstract, Runtime, InternalCall or Imported must have zero RVA, and vice versa. |
There was a problem hiding this comment.
Consider using Verification.FailsPEVerify with { PEVerifyMessage = "..." } instead of just the comment. #Resolved
| """; | ||
| var comp = CreateCompilation(source); | ||
| comp.VerifyEmitDiagnostics( | ||
| // (13,14): error CS0601: The DllImport attribute must be specified on a method marked 'static' and 'extern' |
There was a problem hiding this comment.
// (13,14): error CS0601: The DllImport attribute must be specified on a method marked 'static' and 'extern'
It looks like this error breaks our portability story. No error for classic extension method:
using System.Runtime.InteropServices;
static class E
{
[DllImport("something.dll")]
extern static void M(this int p);
}
``` #Closed
| } | ||
| """; | ||
| var comp = CreateCompilation(source); | ||
| comp.VerifyEmitDiagnostics( |
There was a problem hiding this comment.
I prefer this
This makes the test compile and emit the code twice, which slows the run
| extension(int i) | ||
| { | ||
| extern void M(); | ||
| extern int P { get; set; } |
| } | ||
| """; | ||
| comp = CreateCompilation(source); | ||
| comp.VerifyEmitDiagnostics( |
| } | ||
| """; | ||
| comp = CreateCompilation(source); | ||
| comp.VerifyEmitDiagnostics( |
| } | ||
| """; | ||
| var comp = CreateCompilation(source); | ||
| comp.VerifyEmitDiagnostics(); |
| } | ||
|
|
||
| [Fact] | ||
| public void Extern_08() |
|
Done with review pass (commit 5) #Closed |
4 participants
Addresses some follow-ups from #78968
Relates to test plan #76130