Tweak diagnostics to account for records #46341
Conversation
| "; | ||
| var comp = CreateCompilation(text); | ||
| comp.GetDeclarationDiagnostics().Verify( | ||
| // (2,8): error CS0146: Circular base class dependency involving 'B<A<T>>' and 'A<T>' |
There was a problem hiding this comment.
class [](start = 54, length = 5)
Is this accurate?
There was a problem hiding this comment.
I don't have a better idea. I'm planning to leave this as-is.
|
Done with review pass (iteration 29) #Closed |
| record R : I, Base; | ||
| "; | ||
| CreateCompilation(source).VerifyDiagnostics( | ||
| // (4,15): error CS1722: Base type 'Base' must come before any interfaces |
There was a problem hiding this comment.
Since "base type" includes interfaces, I'll revert to "base class" here as well.
|
@dotnet/roslyn-compiler for a second review. Thanks |
| @@ -742,13 +742,13 @@ | |||
| <value>The type '{0}' has no constructors defined</value> | |||
| </data> | |||
| <data name="ERR_NoNewAbstract" xml:space="preserve"> | |||
| <value>Cannot create an instance of the abstract class or interface '{0}'</value> | |||
| <value>Cannot create an instance of the abstract type '{0}'</value> | |||
There was a problem hiding this comment.
Consider "abstract type or interface" instead. #Closed
| @@ -2007,16 +2007,16 @@ If such a class is used as a base class and if the deriving class defines a dest | |||
| <value>Invalid constraint type. A type used as a constraint must be an interface, a non-sealed class or a type parameter.</value> | |||
| </data> | |||
| <data name="ERR_InstanceMemberInStaticClass" xml:space="preserve"> | |||
| <value>'{0}': cannot declare instance members in a static class</value> | |||
| <value>'{0}': cannot declare instance members in a static type</value> | |||
There was a problem hiding this comment.
Do we support static record? If not, consider reverting this. #Closed
There was a problem hiding this comment.
We don't support static record, but unfortunately we display many cascading diagnostics in those cases, where the original message would be confusing. See StaticRecordWithConstructorAndDestructor for example.
| </data> | ||
| <data name="ERR_StaticBaseClass" xml:space="preserve"> | ||
| <value>'{1}': cannot derive from static class '{0}'</value> | ||
| <value>'{1}': cannot derive from static type '{0}'</value> |
There was a problem hiding this comment.
Please revert if static record is not supported. #Closed
| </data> | ||
| <data name="ERR_ConstructorInStaticClass" xml:space="preserve"> | ||
| <value>Static classes cannot have instance constructors</value> | ||
| <value>Static types cannot have instance constructors</value> |
There was a problem hiding this comment.
Please revert if static record is not supported. #Closed
| </data> | ||
| <data name="ERR_DestructorInStaticClass" xml:space="preserve"> | ||
| <value>Static classes cannot contain destructors</value> | ||
| <value>Static types cannot contain destructors</value> |
There was a problem hiding this comment.
Please revert if static record is not supported. #Closed
| "); | ||
|
|
||
| c.VerifyDiagnostics( | ||
| // (8,12): error CS0060: Inconsistent accessibility: base type 'X<B.C.D.E>' is less accessible than class 'B.C' |
There was a problem hiding this comment.
// [](start = 12, length = 10)
Indenting looks off.
| "; | ||
| var comp = CreateCompilation(text); | ||
| comp.GetDeclarationDiagnostics().Verify( | ||
| // (2,8): error CS0146: Circular base class dependency involving 'B<A<T>>' and 'A<T>' |
There was a problem hiding this comment.
Circular base class [](start = 40, length = 19)
"Circular base type" here and below. #Closed
There was a problem hiding this comment.
The reason for using "base class" is that the spec uses "base type" to encompass the base class and all interfaces.
See https://github.com/dotnet/csharplang/blob/master/spec/classes.md#type-parameter-constraints
So "base type" is incorrect (although intuitive). I'm leaving as-is for now unless we have a better name. #Closed
There was a problem hiding this comment.
The reason for using "base class" is that the spec uses "base type" to encompass the base class and all interfaces.
That sounds fine to me, but the resource string is currently "Circular base type dependency involving '{0}' and '{1}'"
In reply to: 463328299 [](ancestors = 463328299)
There was a problem hiding this comment.
Sorry, got confused. The meaning of "base type" that I referenced is in the context of type constraints.
ghost
modified the milestones:
* upstream/master: (304 commits) Tweak diagnostics to account for records (dotnet#46341) Diagnose precedence inversion in a warning wave (dotnet#46239) Remove PROTOTYPE tag (dotnet#45965) Only run a single pass of NullableWalker per-member (dotnet#46402) Fix crash, and offer "declare as nullable" for tuple fields (dotnet#46437) Simplify contract for RunWithShutdownBlockAsync Fix optprof plugin input check if EditorAdaptersFactoryService gives us a null buffer Cannot assign maybe-null value to TNotNull variable (dotnet#41445) Fix overload resolution to handle binary compat in the face of covariant returns (dotnet#46367) Same failure on Linux Skip some tests on Mac Added search option for inline parameter name hints Spelling tweak docs Improve comment Improve the "not exhaustive" diagnostic in the presence of a when clause. (dotnet#46143) PR feedback Use record keyword to display records (dotnet#46338) remove test that aserts .NET Standard should be prefered over .NET Framework ...
Fixes #45207