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

ValueTuple in desktop 4.6 app clashes with that of mscorlib 4.7 #20235

Closed
jcouv opened this issue Feb 15, 2017 · 12 comments
Closed

ValueTuple in desktop 4.6 app clashes with that of mscorlib 4.7 #20235

jcouv opened this issue Feb 15, 2017 · 12 comments

Comments

@jcouv
Copy link
Member

@jcouv jcouv commented Feb 15, 2017

Repro:

  1. Make a trivial app targeting 4.6 involving tuples (you must add a reference to ValueTuple package)
  2. On a machine with 4.7 installed, try to debug that program
  3. The program runs fine, with modules listed below (ValueTuple.dll from the corefx package included in the app, mscorlib.dll from desktop 4.7)
  4. In the EE (Immediate Window, Watch, QuickWatch) type something that involves tuple syntax
    This will give an error CS8179, because the compiler found two instances of the well-known type (that is ambiguity).

image

(1, 2)
error CS8179: Predefined type 'System.ValueTuple`2' is not defined or imported
t = (1, 2)
error CS8179: Predefined type 'System.ValueTuple`2' is not defined or imported
        static void Main(string[] args)
        {
            (int first, int second) t = (1, 1);
            while(true)
            {
                Thread.Sleep(1000);
                Console.WriteLine(t.first);
                t.first++;
            }
        }

FYI @tmat @VSadov @cston @AlekseyTs

@tmat
Copy link
Member

@tmat tmat commented Feb 15, 2017

A fix would be to include System.ValueTuple.dll in .NET Framework 4.7 that type-forwards all types to mscorlib and add it to GAC. When loading System.ValueTuple the loader would prefer this library over the one in the app dir. So there would be no duplicates loaded.

@tmat
Copy link
Member

@tmat tmat commented Feb 15, 2017

Issue dotnet/roslyn#17157 has the same root cause.

@jcouv
Copy link
Member Author

@jcouv jcouv commented Feb 15, 2017

Discussed with @weshaggard
If we made a change to desktop 4.7 to have type unification to System.ValueTuple.dll version 4.0 (type unification only cares about major and minor version), then we'd introduce another problem.
System.ValueTuple.dll 4.0.1.1 has ITuple, but desktop 4.7 doesn't.
An app that used ITuple on desktop 4.6 would lose it when loaded on a machine with 4.7. (Maybe that isn't so bad?)
I'm going to explore an EE/compiler solution and I'll get back with some findings.

@tmat
Copy link
Member

@tmat tmat commented Feb 15, 2017

Why doesn't 4.7 have ITuple? Can we add it?

@jcouv
Copy link
Member Author

@jcouv jcouv commented Feb 15, 2017

I shiproom rejected the ITuple PR when I made it (a few days past code complete alas), so I doubt they would take it now.
@weshaggard Let me know if I'm being too pessimistic.

@tmat
Copy link
Member

@tmat tmat commented Feb 15, 2017

The shiproom didn't know we have a bigger problem.

@tmat
Copy link
Member

@tmat tmat commented Feb 15, 2017

If the compiler resolved the ambiguity by picking the type from mscorlib (as we do in EEs for other corlib ambiguities), wouldn't we have the same problem with ITuple? That EE wouldn't see ITuple, so expression evaluation would behave differently than the application code. That would be confusing for the user.

@jcouv
Copy link
Member Author

@jcouv jcouv commented Mar 30, 2017

Ping @weshaggard This is the issue I mentioned, which will require work in desktop 4.7.1 (to the GAC and type unification), but not to corefx itself.

@jcouv
Copy link
Member Author

@jcouv jcouv commented Mar 30, 2017

@danmosemsft
Copy link
Member

@danmosemsft danmosemsft commented Apr 18, 2017

@AlexGhiondea is this going to be part of what you're doing?
Also should you move it to the desktop bug tracker?

@AlexGhiondea
Copy link
Member

@AlexGhiondea AlexGhiondea commented Apr 19, 2017

Yes it is. And there is no need for an additional issue -- i am handling this as part of a larger work item.

@ericstj
Copy link
Member

@ericstj ericstj commented Apr 25, 2017

I believe I've also addressed this in the NETStandard.Library.NETFramework support package. Closing this.

@ericstj ericstj closed this Apr 25, 2017
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.