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

Tuple arg names removed from definition #2806

Closed
tamlin-mike opened this issue Oct 20, 2022 · 5 comments
Closed

Tuple arg names removed from definition #2806

tamlin-mike opened this issue Oct 20, 2022 · 5 comments
Labels
Bug Decompiler The decompiler engine itself

Comments

@tamlin-mike
Copy link

Repro code:

namespace Z {
  using System.Collections.Generic;
  class C {
    public void m(List<(string,string)> lst) {
      IEnumerator<(string hello, string there)> it = lst.GetEnumerator();
      {
        var e = it.Current;
        string s = "{e.hello}, {e.there}";
      }
    }
  }
}

When decompiling, the enumerator definition in error becomes

  IEnumerator<(string, string)> ...

i.e. the required named arguments hello and there were erased, which in this scenario is ... not optimal. :-)

When later using the iterator, the decompilation displays it is indeed aware of the names

  string text = "{e.hello}, {e.there}";

I haven't tested the more common case, where the anonymous tuple type is a direct variable. It's entirely possible the problem is purely when used as a generic type argument.

@tamlin-mike tamlin-mike added Bug Decompiler The decompiler engine itself labels Oct 20, 2022
@dgrunwald
Copy link
Member

Your string literal isn't using $, so the names appearing in the literal doesn't mean the decompiler is aware of them.
Tuple element names are stored in an attribute. For local variables, they aren't stored at all in the assembly.

@tamlin-mike
Copy link
Author

sigh I should have known I simplified it too far (using a plain text editor). Thanks for catching it.

The original code I encountered the problem in was slightly more complex, but the main premise of this report holds - the decompiler displayed the given names for the tuple arguments where they were referenced, but didn't include them in the definition.

@siegfriedpammer
Copy link
Member

Would still be nice if you could provide an example. As tuple element names are stored in an attribute, local variables never have custom element names in the IL, because there is no way to attach attributes to local variables.

@tamlin-mike
Copy link
Author

My mental model was not aligned with actual design+implementation. Turns out it was in a more complex scenario of generated code, with a hidden generated capture-class and an inner function. Now I'm not the least surprised it slipped by. ILSpy is so good nowadays it makes this kind of code look so easy you often forget it's actually a mess of compiler generates state machines and hidden classes ad infinitum. :-)

Have a look at
Assembly: Microsoft.VisualStudio.Shell.UI.Internal.dll (I looked at version 16.0.31727.243) (*)
NS: Microsoft.VisualStudio.PlatformUI.Packages.Search.Package
Class: SearchPackage
Method: private void SaveMostRecentQueries(Stream stream)

Of importance is the top of the decompiled method, specifically queries:
if (!TryGetOptionValue<ImmutableArray<(string, string)>>("SearchMostRecentQueriesList", out var queries))
and later the use inside the inner Writer function
...queries[i].resultType, ...queries[i].query

The named tuple typedefinition is tucked away inside a generic type argument to the ImmutableArray, inside a compiler-generated class, and only by selecting that field in the tree view do we see the details we're after. In IL we see the field ctor invocation but unfortunately only as a hex array (maybe it could be considered to display it as a string in a comment?), but in C# view we finally see the names in all their glory.

Yeah... not exactly an obvious hiding place. Now I'm even less suprised this slipped by.

(*) I have hope (and strong reasons to believe) that code remains even in the later version(s) you use. If not, the file is inside the package named Microsoft.VisualStudio.MinShell,version=16.11.31727.386, in its payload.vsix.

@siegfriedpammer
Copy link
Member

using System;
using System.Collections.Generic;

public class C
{
	T Get<T>() => default;
	bool TryGet<T>(out T result) { result = default; return true; }
	
    public void M()
	{
        Dictionary<int, (int A, string B)> data = new();
        
        Test();
        
        int Test()
        {
            return data[0].A;
        }
    }
    
    public void M2()
	{
        var data = Get<Dictionary<int, (int A, string B)>>();
        
        Test();
        
        int Test()
        {
            return data[0].A;
        }
    }
    
    public void M3()
	{
        TryGet<Dictionary<int, (int A, string B)>>(out var data);
        
        Test();
        
        int Test()
        {
            return data[0].A;
        }
    }
}

M3 reproduces the problem.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Decompiler The decompiler engine itself
Projects
None yet
Development

No branches or pull requests

3 participants