File types emit #61646
File types emit #61646
Conversation
3d9a8ac to
9a1c962
Compare
src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.cs
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
|
The EnC tests I've added work fine, but the manual end-to-end test doesn't. A simple edit of an existing method in a file type yields the following diagnostic in the Error List: Will have to look tomorrow to see how to properly triage this. Maybe just attaching a debugger will help :) edit: addressing this issue in #62215 |
This comment was marked as resolved.
This comment was marked as resolved.
| @@ -326,6 +330,34 @@ internal virtual NamedTypeSymbol LookupMetadataType(ref MetadataTypeName emitted | |||
| } | |||
|
|
|||
| Done: | |||
| if (isTopLevel | |||
There was a problem hiding this comment.
I agree that handling lookup for error cases quickly gets complicated :-/ Not blocking this PR, but it would be good to check what Aleksey thinks on supporting "correct" behavior of public APIs in error cases. #Closed
There was a problem hiding this comment.
I agree that handling lookup for error cases quickly gets complicated :-/ Not blocking this PR, but it would be good to check what Aleksey thinks on supporting "correct" behavior of public APIs in error cases.
It is not clear what specific scenarios we are talking about. Could you please elaborate?
There was a problem hiding this comment.
@AlekseyTs The scenario is GetTypeByMetadataName on a nested file-type (class C { file class D { } }), ie. an error scenario.
There was a problem hiding this comment.
The scenario is
GetTypeByMetadataNameon a nested file-type (``class C { file class D { } }```), ie. an error scenario.
I think in this case the file modifier on a nested type should be ignored and class D should be treated as a regular nested type. In other words, I think that the best implementation strategy here will be to not have nested file types in the symbol model, under no condition.
There was a problem hiding this comment.
Perfect. That's the approach Rikki took and it makes things simpler. Thanks
@RikkiGibson If we take that approach, let's verify the symbol for class D has !IsFile then.
There was a problem hiding this comment.
We're going to add an item to test plan and ensure the 'IsFile' API behaves as described here in a subsequent PR.
|
|
||
| var types = comp.GetTypesByMetadataName("<>F0__C"); | ||
| Assert.Equal(2, types.Length); | ||
| Assert.Equal(firstIsMetadataReference ? "C@<tree 0>" : "<>F0__C", types[0].ToTestDisplayString()); |
There was a problem hiding this comment.
Added test plan item
| @@ -326,6 +330,35 @@ internal virtual NamedTypeSymbol LookupMetadataType(ref MetadataTypeName emitted | |||
| } | |||
|
|
|||
| Done: | |||
| if (isTopLevel | |||
| && scope is not PENamespaceSymbol | |||
| && (emittedTypeName.ForcedArity == -1 || emittedTypeName.ForcedArity == emittedTypeName.InferredArity) | |||
There was a problem hiding this comment.
Added test plan item
Related to #60819
This primarily does the name mangling. In this PR I chose the format
<ContainingFileName>F0__OriginalTypeName. We do name mangling through the same mechanism which adds`1suffixes to generic type names.