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

Find references, go to definition on ConditionalWeakTable.GetValue #45437

Closed
jasonmalinowski opened this issue Jun 24, 2020 · 4 comments · Fixed by #45594
Closed

Find references, go to definition on ConditionalWeakTable.GetValue #45437

jasonmalinowski opened this issue Jun 24, 2020 · 4 comments · Fixed by #45594
Assignees
Labels
4 - In Review A fix for the issue is submitted for review. Area-IDE Bug
Milestone

Comments

@jasonmalinowski
Copy link
Member

Version Used: 3.7.0-4.20323.4+11e94bdd96f1500f7762b923728e4d322a43bdc7

Steps to Reproduce:

  1. Go to some use of ConditionalWeakTable.GetValue, for example
    return s_bufferToWorkspaceRegistrationMap.GetValue(textContainer, s_createRegistration);
  2. Invoke Find References on the GetValue.

Expected Behavior: It works.

Actual Behavior: I'm told there's no references.

Fun Fact: go to definition doesn't work either:

---------------------------
Microsoft Visual Studio
---------------------------
Cannot navigate to the symbol under the caret.
---------------------------
OK   
---------------------------

Oddly I can go to other members of ConditionalWeakTable.

@CyrusNajmabadi
Copy link
Member

Wacky. I repro this, but cannot explain it either. Will have to debug through.

@CyrusNajmabadi
Copy link
Member

So i have found the issue, but i cannot come up with a self-contained repro for it.

Specifically, consider the following code in a .netcore 3.1 project:

using System.Runtime.CompilerServices;

class c
{
    private readonly ConditionalWeakTable<string, string>.CreateValueCallback s_get;

    void Goo()
    {
        ConditionalWeakTable<string, string> s = null;
        s.GetValue("", s_get);  //goto-def on GetValue
    }
}

Setting a breakpoint here:

var displayParts = GetDisplayParts(definition);

We see we have an IMethodSymbol for: System.Runtime.CompilerServices.ConditionalWeakTable<TKey, TValue>.GetValue(TKey, System.Runtime.CompilerServices.ConditionalWeakTable<TKey, TValue>.CreateValueCallback)

However, if we check the following: ((IMethodSymbol)definition).Parameters[1].Type.IsDefinition we see a result of false. This corresponds to the ConditionalWeakTable<TKey, TValue>.CreateValueCallback parameter of GetValue. Effectively, despite this symbol referring to a nested type in generic type instantiated with its own type parameters, it says it is not the definition, even though it it really should be true.

Note that we don't do anything fancy here to get this IMethodSymbol. It's just a plain SemanticModel.GetSymbolInfo.

I cannot make out why or what causes us to get teh bogus one back in the IDE. However, this ends up breaking things later on down the line for us as we end up getting confused between this type, and the equivalent type (except this time with .IsDefinition == true).

I'll need help with this as it's unclear to me what is going on.

@CyrusNajmabadi
Copy link
Member

@gafter @jcouv Sorry i couldn't find a simple test-repro for this. In tests it always seems to work properly. However, reproing with IDE is really simple, and seems to demonstrate the problem immediately. Could def use some help here with this!

@gafter
Copy link
Member

gafter commented Jul 1, 2020

Per @CyrusNajmabadi This appears to be a difference in the nullability of the original definition and this reference. Both the top-level nullability and the nullability of the type arguments differ (one is oblivious on the TKey and top level, the other unannotated).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - In Review A fix for the issue is submitted for review. Area-IDE Bug
Projects
None yet
5 participants