Description

Jason Freeman (JsonFreeman)

opened on Jul 18, 2014

Issue body actions

Now that we have a heuristic algorithm for infinitely expanding generics, we can go back to comparing signatures using contextual signature instantiation. For example:

var f: (s: string) => number;
var g: <T>(s: T) => T;
f = g;
g = f;

Both of these should ideally fail. The reason we could not do this earlier was because we could not detect infinitely expanding generics properly. But the current algorithm would prevent us from blowing up in the cases where the previous algorithm failed.

Activity

1. Jason Freeman (JsonFreeman)

   added

   Suggestion

   on Jul 18, 2014
2. JsonFreeman commented on Jul 22, 2014

   

   Jason Freeman (JsonFreeman)

   on Jul 22, 2014

   ContributorAuthor

   More actions

   Here is another very surprising example:

   function arrayize<T extends String>(x: T) {
       return [x];
   }
   
   var xs = [1, 2, 3];
   var z = xs.map(arrayize);
   var s: string = z;

   The type argument to map is inferred to be String[], so we are trying to pass arrayize into something that takes a number => String[]. However, the assignability still succeeds because arrayize is instantiated to any => any[], which is compatible with number => String[]. Anders Hejlsberg (@ahejlsberg) and I looked at this, and it was surprising to both of us that this was allowed.
3. Ryan Cavanaugh (RyanCavanaugh)

   added

   Needs Proposal

   on Jul 28, 2014
4. RyanCavanaugh commented on Jul 28, 2014

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Jul 28, 2014

   Member

   More actions

   Looking for formal description + a rundown of potential changes
5. myitcv commented on Apr 8, 2016

   

   Paul Jolly (myitcv)

   on Apr 8, 2016

   More actions

   Ryan Cavanaugh (@RyanCavanaugh) Jason Freeman (@JsonFreeman) thanks for the responses on #241

   What needs to happen to move this forward?
6. RyanCavanaugh commented on Apr 8, 2016

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Apr 8, 2016

   Member

   More actions

   I believe it's basically an expert-level "bug fix" in the checker code somewhere, plus some spec language. I'm not familiar with how contextual signature instantiation actually works so I don't have any more details.
7. JsonFreeman commented on Apr 9, 2016

   

   Jason Freeman (JsonFreeman)

   on Apr 9, 2016

   ContributorAuthor

   More actions

   Paul Jolly (@myitcv) The fix would be to replace the two calls to getErasedSignature in compareSignaturesRelated. Instead you would call instantiateSignatureInContextOf(source, target, undefined). I believe this would return an instantiated version of source that would then be assignable to target (in terms of parameters and return type).

   There are a few other things that would need to be worked out though. You'd have to detect an instantiation failure, which would indicate that the signatures are not compatible. Another thing to consider is the type parameter constraints of the source signature, which is trickier than it used to be because of the new F-bounded polymorphism feature.

   Contextual signature instantiation was indeed used to compare signatures before 1.0. We switched to a loose check for a few reasons. Chiefly, many users found it too strict - there were frequent errors on code that seems okay. And it is a more expensive check, particularly when comparing a generic signature member of an infinitely expanding generic type. These are two caveats to watch out for when trying this fix.
8. myitcv commented on Apr 9, 2016

   

   Paul Jolly (myitcv)

   on Apr 9, 2016

   More actions

   Jason Freeman (@JsonFreeman) thanks for the notes, almost certainly well beyond my capabilities to even think about contributing to this one.

   When combined with #7234, this is biting us pretty hard at the moment. So any progress on both issues would be massively appreciated!
9. mhegazy commented on May 18, 2017

   

   Mohamed Hegazy (mhegazy)

   on May 18, 2017

   Contributor

   More actions

   This is essentially the same as #5616
10. Anders Hejlsberg (ahejlsberg)

    added

    FixedA PR has been merged for this issueA PR has been merged for this issue

    on Jun 8, 2017
11. Anders Hejlsberg (ahejlsberg)

    closed this as completedin #16368on Jun 12, 2017
12. Mohamed Hegazy (mhegazy)

    added this to the TypeScript 2.4.1 milestone on Jun 12, 2017
13. microsoft

    locked and limited conversation to collaborators on Jun 18, 2018

Issue actions