Fix opaque types resulting from projections in function signature

[Aaron1011]

When we normalize the types in a function signature, we may end up
resolving a projection to an opaque type (e.g. Self::MyType when
we have type MyType = impl SomeTrait). When the projection is
resolved, we will instantiate the generic parameters into fresh
inference variables.

While we do want to normalize projections to opaque types, we don't want
to replace the explicit generic parameters (e.g. T in impl MyTrait<T>) with inference variables. We want the opaque type in the
function signature to be eligible to be a defining use of that opaque
type - adding inference variables prevents this, since the opaque type
substs now appears to refer to some specific type, rather than a generic
type.

To resolve this issue, we inspect the opaque types in the function
signature after normalization. Any inference variables in the substs are
replaced with the corresponding generic parameter in the identity substs
(e.g. T in impl MyTrait<T>). Note that normalization is the only way
that we can end up with inference variables in opaque substs in a
function signature - users have no way of getting inference variables
into a function signature.

Note that all of this refers to the opaque type (ty::Opaque) and its
subst - not to the underlying type.

Fixes #59342

[rust-highfive]

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

[Centril]

cc @varkor @nikomatsakis

[davidtwco]

LGTM, might want to have someone else check over it though.

[davidtwco]

r? @varkor

[Aaron1011]

This PR ended up fixing (!) the issue-58887 test, as well as making the issue-60564 error later on. I took the opportunit to improve the error message, as it didn't actually mention which generic parameter was causing the issue.

[varkor]

We could improve this error. (Also, I think define should be determine.) Maybe with a help message like:

uses of an opaque type must fully determine it, but here `Foo` is only determined for `V = <T as TraitWithAssoc>::Assoc`

It took me a couple of tries to work out what the error message was saying before.

[Aaron1011]

@varkor: Your proposed error message is backward - V is the parameter that is not determined (because it's a concrete type instead of a generic parameter).

[Aaron1011]

While I think there's room to improve this error, this modification provides strictly more information. I think further improvements should go in another PR, since this PR is really about changing when error messages occur, not the actual contente.

[varkor]

Okay, but let's open an issue for improving this.

[varkor]

Opened #66723.

[varkor]

When the projection is resolved, we will instantiate the generic parameters into fresh inference variables.

Just to clarify, normalisation is replacing generic parameters with inference variables, and then this pass is doing the opposite? Can we not just avoid replacing such generic parameters in the first place?

[Aaron1011]

Can we not just avoid replacing such generic parameters in the first place?

@varkor: Not easily. These inference variables are created when we select the matching impl, which happens behind several layers of indrection (normalize -> project -> select).

As far I as can tell, this is the only place where we don't want the usual behavior of creating new inference variables. I think it makes more sense to fix it up here, rather than adding another flag to SelectionContext.

[varkor]

Is there a possibility that there were previously some inference variables that did not come from replaced generic parameters, but we're now going to replace with new generic parameters?

[Aaron1011]

@varkor: This PR only replaces inference variables that occur directly in the substs of an opaque type, which by definition corresponds to a generic parameter.

[Centril]

@Aaron1011 @rust-lang/compiler-contributors For future reference, when a PR implements a particular (or more) feature, I would love it if you could ensure if the appropriate F-$feature_gate label is applied (or if such a label doesn't exist yet for some reason, make one). This system provides an easy way to track the history of a feature's implementation and will facilitate auditing by the language team / whoever writes the report.

[varkor]

This PR only replaces inference variables that occur directly in the substs of an opaque type, which by definition corresponds to a generic parameter.

Okay, this seems reasonable. Could you add some of the explanation you gave in your comments here to the comment in the code, just to give more context to someone coming across it without the background? Then I'm happy to go with this after the remaining comments have been addressed.

[Aaron1011]

@varkor: I've added some of my explanation to the comments, and added a worked out example. This should now be ready to merge.

[bors]

📌 Commit df3f338 has been approved by varkor

[bors]

⌛ Testing commit df3f338 with merge eef352e0f85460c42ae9bf53b0a9f4070c054634...

[rust-highfive]

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[bors]

💔 Test failed - checks-azure

[Aaron1011]

@varkor: This looks like a spurious failure

[varkor]

@bors retry

[bors]

⌛ Testing commit df3f338 with merge 52ab26279852c6ee751cbdc0cd820a0ab2dc5b34...

[rust-highfive]

Your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2019-11-25T12:52:29.8663326Z 
2019-11-25T12:52:29.8664364Z 
2019-11-25T12:52:29.8668828Z 
2019-11-25T12:52:29.8674851Z 
2019-11-25T12:52:29.8677003Z (e.g. `T` in `impl MyTrait<T>`). Note that normalization is the only way
2019-11-25T12:52:29.8686258Z AGENT_DISABLELOGPLUGIN_TESTFILEPUBLISHERPLUGIN=true
2019-11-25T12:52:29.8686371Z AGENT_DISABLELOGPLUGIN_TESTRESULTLOGPLUGIN=true
2019-11-25T12:52:29.8686469Z AGENT_HOMEDIRECTORY=C:\agents\2.160.1
2019-11-25T12:52:29.8686541Z AGENT_ID=520
---
2019-11-25T12:52:29.8701259Z MAVEN_OPTS=-Xms256m
2019-11-25T12:52:29.8701364Z MSDEPLOY_HTTP_USER_AGENT=VSTS_d439fc94-e01f-4249-b63e-d8392bc0247c_build_10_0
2019-11-25T12:52:29.8701457Z MSMPI_BIN=C:\Program Files\Microsoft MPI\Bin\
2019-11-25T12:52:29.8701544Z MSYSTEM=MINGW64
2019-11-25T12:52:29.8701627Z MyTrait<T>`) with inference variables. We want the opaque type in the
2019-11-25T12:52:29.8701794Z NPM_CONFIG_PREFIX=C:\npm\prefix
2019-11-25T12:52:29.8701881Z NUMBER_OF_PROCESSORS=2
2019-11-25T12:52:29.8701983Z Note that all of this refers to the opaque type (ty::Opaque) and its
2019-11-25T12:52:29.8702062Z OS=Windows_NT
---
2019-11-25T12:52:29.8716554Z function signature - users have no way of getting inference variables
2019-11-25T12:52:29.8716684Z function signature to be eligible to be a defining use of that opaque
2019-11-25T12:52:29.8716780Z inference variables.
2019-11-25T12:52:29.8716872Z into a function signature.
2019-11-25T12:52:29.8716968Z replaced with the corresponding generic parameter in the identity substs
2019-11-25T12:52:29.8717092Z resolved, we will instantiate the generic parameters into fresh
2019-11-25T12:52:29.8717214Z resolving a projection to an opaque type (e.g. `Self::MyType` when
2019-11-25T12:52:29.8717322Z signature after normalization. Any inference variables in the substs are
2019-11-25T12:52:29.8717428Z subst - *not* to the underlying type.
2019-11-25T12:52:29.8717526Z substs now appears to refer to some specific type, rather than a generic
2019-11-25T12:52:29.8717649Z that we can end up with inference variables in opaque substs in a
2019-11-25T12:52:29.8717754Z to replace the explicit generic parameters (e.g. `T` in `impl
2019-11-25T12:52:29.8717878Z type - adding inference variables prevents this, since the opaque type
2019-11-25T12:52:29.8717982Z type.
2019-11-25T12:52:29.8718153Z we have `type MyType = impl SomeTrait`). When the projection is
2019-11-25T12:52:29.8718343Z disk usage:
2019-11-25T12:52:29.9752397Z Filesystem            Size  Used Avail Use% Mounted on
2019-11-25T12:52:29.9755428Z C:/Program Files/Git  256G  140G  116G  55% /
2019-11-25T12:52:29.9756717Z D:                     14G  2.0G   13G  15% /d
---
2019-11-25T12:53:05.1506131Z  84  480M   84  408M    0     0  14.0M      0  0:00:34  0:00:28  0:00:06 22.9M
2019-11-25T12:53:06.6224617Z  90  480M   90  433M    0     0  14.4M      0  0:00:33  0:00:29  0:00:04 22.9M
2019-11-25T12:53:06.7385064Z  95  480M   95  456M    0     0  14.5M      0  0:00:33  0:00:31  0:00:02 23.0M
2019-11-25T12:53:06.7396976Z  95  480M   95  460M    0     0  14.5M      0  0:00:32  0:00:31  0:00:01 22.6M
2019-11-25T12:53:06.7397329Z curl: (56) OpenSSL SSL_read: SSL_ERROR_SYSCALL, errno 10054
2019-11-25T12:53:06.7418300Z 
2019-11-25T12:53:06.7418673Z gzip: stdin: unexpected end of file
2019-11-25T12:53:06.7431177Z tar: Unexpected EOF in archive
2019-11-25T12:53:06.7433506Z tar: Unexpected EOF in archive
2019-11-25T12:53:06.7433845Z tar: Error is not recoverable: exiting now
2019-11-25T12:53:06.7485083Z 
2019-11-25T12:53:06.7572428Z ##[error]Bash exited with code '2'.
2019-11-25T12:53:06.7779867Z ##[section]Starting: Checkout
2019-11-25T12:53:06.7879424Z ==============================================================================
2019-11-25T12:53:06.7879531Z Task         : Get sources
2019-11-25T12:53:06.7879627Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[bors]

💔 Test failed - checks-azure

[Aaron1011]

@varkor: Another spurious failure

[varkor]

@bors retry

[bors]

⌛ Testing commit df3f338 with merge 483a83b...

[bors]

☀️ Test successful - checks-azure
Approved by: varkor
Pushing 483a83b to master...