-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Support unbound generic types in 'nameof' operator. #75368
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
Changes from all commits
b363549
f1f276b
d5d1423
f16971d
6d9a068
b4ff542
092b89c
d0b2d12
2d0155a
db46266
074bd23
52dd592
08e46f4
5f1fd05
bbdb37e
2198c85
ce2286a
116d579
da8e09e
3147497
1df770b
10b1414
5a695dc
4fcadd9
7aff344
f006bbb
74a3fe0
a671848
ae6727f
74bdd91
7a6328a
cf0df29
5af3f7a
48688a5
d0b7fa5
13e7eba
1e6366f
739a4ef
401ec47
9feec81
6f6e987
9497a5e
7ed0bfe
30c1a45
efeea4c
f77d4d0
0a3e50a
21d4a4e
0be4c20
a0e8e13
4d19b86
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Collections.Generic; | ||
| using Microsoft.CodeAnalysis.CSharp.Symbols; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
| using Roslyn.Utilities; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.CSharp | ||
|
|
@@ -19,19 +21,25 @@ namespace Microsoft.CodeAnalysis.CSharp | |
| /// </summary> | ||
| internal sealed class NameofBinder : Binder | ||
| { | ||
| private readonly SyntaxNode _nameofArgument; | ||
| private readonly ExpressionSyntax _nameofArgument; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This always had to be an ExpressionSyntax. Making strongly typed and to match the sig of OpenTypeVisitor.VisitThis |
||
| private readonly WithTypeParametersBinder? _withTypeParametersBinder; | ||
| private readonly Binder? _withParametersBinder; | ||
| private ThreeState _lazyIsNameofOperator; | ||
|
|
||
| internal NameofBinder(SyntaxNode nameofArgument, Binder next, WithTypeParametersBinder? withTypeParametersBinder, Binder? withParametersBinder) | ||
| private readonly Dictionary<GenericNameSyntax, bool>? _allowedMap; | ||
|
|
||
| internal NameofBinder(ExpressionSyntax nameofArgument, Binder next, WithTypeParametersBinder? withTypeParametersBinder, Binder? withParametersBinder) | ||
| : base(next) | ||
| { | ||
| _nameofArgument = nameofArgument; | ||
| _withTypeParametersBinder = withTypeParametersBinder; | ||
| _withParametersBinder = withParametersBinder; | ||
| OpenTypeVisitor.Visit(nameofArgument, out _allowedMap); | ||
| } | ||
|
|
||
| protected override bool IsUnboundTypeAllowed(GenericNameSyntax syntax) | ||
| => _allowedMap != null && _allowedMap.TryGetValue(syntax, out bool allowed) && allowed; | ||
|
|
||
| private bool IsNameofOperator | ||
| { | ||
| get | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Collections.Generic; | ||
| using Microsoft.CodeAnalysis.CSharp.Syntax; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.CSharp; | ||
|
|
||
| internal partial class Binder | ||
| { | ||
| /// <summary> | ||
| /// This visitor walks over a type expression looking for open types. | ||
| /// | ||
| /// Open types are allowed if an only if: | ||
| /// <list type="number"> | ||
| /// <item>There is no constructed generic type elsewhere in the visited syntax; and</item> | ||
| /// <item>The open type is not used as a type argument or array/pointer/nullable element type.</item> | ||
| /// </list> | ||
| /// | ||
| /// Open types can be used both in <c>typeof(...)</c> and <c>nameof(...)</c> expressions. | ||
| /// </summary> | ||
| protected sealed class OpenTypeVisitor : CSharpSyntaxVisitor | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extracted out of TypeOfBinder so it can be used by TypeOfBinder and NameOfBinder. |
||
| { | ||
| private Dictionary<GenericNameSyntax, bool>? _allowedMap; | ||
| private bool _seenConstructed; | ||
|
|
||
| /// <param name="typeSyntax">The argument to typeof.</param> | ||
| /// <param name="allowedMap"> | ||
| /// Keys are GenericNameSyntax nodes representing unbound generic types. | ||
| /// Values are false if the node should result in an error and true otherwise. | ||
| /// </param> | ||
| public static void Visit(ExpressionSyntax typeSyntax, out Dictionary<GenericNameSyntax, bool>? allowedMap) | ||
| { | ||
| OpenTypeVisitor visitor = new OpenTypeVisitor(); | ||
| visitor.Visit(typeSyntax); | ||
| allowedMap = visitor._allowedMap; | ||
| } | ||
|
|
||
| public override void VisitGenericName(GenericNameSyntax node) | ||
| { | ||
| SeparatedSyntaxList<TypeSyntax> typeArguments = node.TypeArgumentList.Arguments; | ||
| if (node.IsUnboundGenericName) | ||
| { | ||
| _allowedMap ??= new Dictionary<GenericNameSyntax, bool>(); | ||
| _allowedMap[node] = !_seenConstructed; | ||
| } | ||
| else | ||
| { | ||
| _seenConstructed = true; | ||
| foreach (TypeSyntax arg in typeArguments) | ||
| { | ||
| Visit(arg); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public override void VisitQualifiedName(QualifiedNameSyntax node) | ||
| { | ||
| bool seenConstructedBeforeRight = _seenConstructed; | ||
|
|
||
| // Visit Right first because it's smaller (to make backtracking cheaper). | ||
| Visit(node.Right); | ||
|
|
||
| bool seenConstructedBeforeLeft = _seenConstructed; | ||
|
|
||
| Visit(node.Left); | ||
|
|
||
| // If the first time we saw a constructed type was in Left, then we need to re-visit Right | ||
| if (!seenConstructedBeforeRight && !seenConstructedBeforeLeft && _seenConstructed) | ||
| { | ||
| Visit(node.Right); | ||
| } | ||
| } | ||
|
|
||
| public override void VisitAliasQualifiedName(AliasQualifiedNameSyntax node) | ||
| { | ||
| Visit(node.Name); | ||
| } | ||
|
|
||
| public override void VisitArrayType(ArrayTypeSyntax node) | ||
| { | ||
| _seenConstructed = true; | ||
| Visit(node.ElementType); | ||
| } | ||
|
|
||
| public override void VisitPointerType(PointerTypeSyntax node) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure. @333fred do function pointers need special handling here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: i've added some typeof and nameof tests taht exercise func-ptrs. The results seem expected to me. But i wouldn't mind SMEs here validating.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've played with the change a bit and looked through the logic, and I can't see a scenario where it should matter; since we use
I think that covers all the possibilities, so I don't expect there to be any issues. However, @CyrusNajmabadi, please add a test with 3 generics used like this:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @333fred tests added. test results seemed sensible to me, so i'm moving forward with submitting this. tnx. |
||
| { | ||
| _seenConstructed = true; | ||
| Visit(node.ElementType); | ||
| } | ||
|
|
||
| public override void VisitNullableType(NullableTypeSyntax node) | ||
| { | ||
| _seenConstructed = true; | ||
| Visit(node.ElementType); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,92 +36,5 @@ protected override bool IsUnboundTypeAllowed(GenericNameSyntax syntax) | |
| bool allowed; | ||
| return _allowedMap != null && _allowedMap.TryGetValue(syntax, out allowed) && allowed; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// This visitor walks over a type expression looking for open types. | ||
| /// Open types are allowed if an only if: | ||
| /// 1) There is no constructed generic type elsewhere in the visited syntax; and | ||
| /// 2) The open type is not used as a type argument or array/pointer/nullable | ||
| /// element type. | ||
| /// </summary> | ||
| private class OpenTypeVisitor : CSharpSyntaxVisitor | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moved to sibling type so it could be shared. |
||
| { | ||
| private Dictionary<GenericNameSyntax, bool> _allowedMap; | ||
| private bool _seenConstructed; | ||
|
|
||
| /// <param name="typeSyntax">The argument to typeof.</param> | ||
| /// <param name="allowedMap"> | ||
| /// Keys are GenericNameSyntax nodes representing unbound generic types. | ||
| /// Values are false if the node should result in an error and true otherwise. | ||
| /// </param> | ||
| public static void Visit(ExpressionSyntax typeSyntax, out Dictionary<GenericNameSyntax, bool> allowedMap) | ||
| { | ||
| OpenTypeVisitor visitor = new OpenTypeVisitor(); | ||
| visitor.Visit(typeSyntax); | ||
| allowedMap = visitor._allowedMap; | ||
| } | ||
|
|
||
| public override void VisitGenericName(GenericNameSyntax node) | ||
| { | ||
| SeparatedSyntaxList<TypeSyntax> typeArguments = node.TypeArgumentList.Arguments; | ||
| if (node.IsUnboundGenericName) | ||
| { | ||
| if (_allowedMap == null) | ||
| { | ||
| _allowedMap = new Dictionary<GenericNameSyntax, bool>(); | ||
| } | ||
| _allowedMap[node] = !_seenConstructed; | ||
| } | ||
| else | ||
| { | ||
| _seenConstructed = true; | ||
| foreach (TypeSyntax arg in typeArguments) | ||
| { | ||
| Visit(arg); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public override void VisitQualifiedName(QualifiedNameSyntax node) | ||
| { | ||
| bool seenConstructedBeforeRight = _seenConstructed; | ||
|
|
||
| // Visit Right first because it's smaller (to make backtracking cheaper). | ||
| Visit(node.Right); | ||
|
|
||
| bool seenConstructedBeforeLeft = _seenConstructed; | ||
|
|
||
| Visit(node.Left); | ||
|
|
||
| // If the first time we saw a constructed type was in Left, then we need to re-visit Right | ||
| if (!seenConstructedBeforeRight && !seenConstructedBeforeLeft && _seenConstructed) | ||
| { | ||
| Visit(node.Right); | ||
| } | ||
| } | ||
|
|
||
| public override void VisitAliasQualifiedName(AliasQualifiedNameSyntax node) | ||
| { | ||
| Visit(node.Name); | ||
| } | ||
|
|
||
| public override void VisitArrayType(ArrayTypeSyntax node) | ||
| { | ||
| _seenConstructed = true; | ||
| Visit(node.ElementType); | ||
| } | ||
|
|
||
| public override void VisitPointerType(PointerTypeSyntax node) | ||
| { | ||
| _seenConstructed = true; | ||
| Visit(node.ElementType); | ||
| } | ||
|
|
||
| public override void VisitNullableType(NullableTypeSyntax node) | ||
| { | ||
| _seenConstructed = true; | ||
| Visit(node.ElementType); | ||
| } | ||
| } | ||
| } | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.