Suppress fallback and ambiguity errors by nikomatsakis · Pull Request #32258 · rust-lang/rust

nikomatsakis · 2016-03-15T08:55:35Z

If the infcx has observed other errors, then suppress both default type
parameter fallback (which can be unreliable, as the full constraint set
is not available) and errors related to unresovled
variables (annoyingly, integer type variables cannot currently be
unified with error, so that has to be a separate mechanism). Also add a
flag to infcx to allow us to independently indicate when we have
observed an error and hence should trigger this suppression mode.

Fixes#31997

cc @alexcrichton
r? @arielb1

arielb1 · 2016-03-15T10:29:03Z

When should this be hit? I can think of this causing damage in snapshots.

@arielb1
"causing damage"
Yes, we could remove it. I don't think it's needed -- rather, we should set the flag at the source of the errors, as I later did. (I could also assert when setting the flag that we are not in a snapshot, just to be safe.)

alexcrichton · 2016-03-15T14:57:26Z

🀄

nikomatsakis · 2016-03-15T19:00:17Z

@arielb1

Casts can resolve types.

Do you see this as a problem for this patch? Can you elaborate?

I would rather change the type_is_known_to_be_sized path (both here and in the other place) to be less violent.

Hmm, ok. What would you like it to do? Just assume true?

Why are integer variables a problem here (maybe we can change ty::cast to be more allowing?).

Because I can't unify them with ty-error.

nikomatsakis · 2016-04-05T22:59:21Z

@arielb1 I'm not really sure where you are in your review here. Do you think I should change something? If so, what you would you prefer? (Or if you're just busy, that's fine too.)

arielb1 · 2016-04-06T20:48:15Z

@nikomatsakis

I would prefer to just treat type_is_known_to_be_sized as true if we are checking whether _: Sized and there are errors (for example, () as *const _ should still be an error) and to have ty::cast treat integer/float variables as actual integer/floats.

If you are doing your best attempt to break out of inference, why not just do it?

I was working on a patch that didn't do select_all_obligations_or_error but rather tried to report more specific errors if there were unresolved type variables. That's it, before I became somewhat busy.

Isn't cast checking the only thing downstream of default obligation? And I think we can solve it by being more careful with type_is_known_to_be_sized. And then we can figure out how much undefined-variable-error-reporting we want to do.

arielb1 · 2016-04-06T20:50:22Z

I was also thinking about propagating TyErr, at least in method selection and for closure arguments to invalid functions. Maybe ignoring "the type of this variable must be known in this context" is more reliable and does not hide too much errors.

If the infcx has observed other errors, then suppress both default type parameter fallback (which can be unreliable, as the full constraint set is not available) and errors related to unresovled variables (annoyingly, integer type variables cannot currently be unified with error, so that has to be a separate mechanism). Also add a flag to `infcx` to allow us to independently indicate when we have observed an error and hence should trigger this suppression mode.

It is odd to have this logic strewn about. This also means that all calls to `type_is_known_to_be_sized` are encapsulated in the cast code, in case we want to update that logic.

nikomatsakis · 2016-04-12T18:07:13Z

@arielb1 so I removed that error suppression logic out of the casts. It didn't seem to cause any new spurious errors. I decided against modifying the logic for when a type is considered sized since it didn't seem necessary. I've also gotten make check passing and rebased. Take another look and tell me what you think. Given the changes to the errors (lots of duplicates removed) this seems like a positive step to me, and I think it no longer has the invasive quality you were concerned about.

arielb1 · 2016-04-14T09:11:26Z

Does this fix let x: *const u32 = 0 as _;? If so, add an rpass test.

arielb1 · 2016-04-14T09:13:52Z

Eh, it shouldn't. Maybe put that fix in a separate PR instead?

arielb1 · 2016-04-14T09:18:57Z

+ // For better error messages, we try to check whether the
+ // target type is known to be sized now (we will also check
+ // later, once inference is more complete done).
+ if !fcx.type_is_known_to_be_sized(cast_ty, span) {


What is constructor doing randomly emitting error messages?
BTW, why do we need the early error message? It causes compilation errors in some cases.

@arielb1
What is constructor doing randomly emitting error messages?
As you well know, a constructor is just another function... seems like as good a time as any. And the return type (in particular, the Result with ErrorReported) clearly indicates that an error is reported.
My thinking was basically that instantiating a cast check does some checks and returns, potentially, further checks that may need to be done later. I'm happy to rename.
BTW, why do we need the early error message? It causes compilation errors in some cases.
Yes, this doesn't seem optimal, I agree. I was just preserving the existing behavior -- but it seems like it could go wrong, if only in unusual situations like x as _ or x as Bar<_>
I originally moved this check to be done at the end, but then you get derived errors later. For example:
let x = foo as SomeTrait;
will give an error because the type of x is SomeTrait, which is not Sized. (Actually, it then later gives a error about "cast to unsized type".)
Probably the best thing would be to rewrite type_is_known_to_be_sized to return "yes, no, maybe" and only error out on no. (Or, for simplicity, just check specifically for casts to traits or slices, and leave the more general function for the end.)

nikomatsakis · 2016-04-14T13:59:04Z

@arielb1 pushed a new commit that tweaks the approach. Unfortunately, this doesn't seem to be quite enough to make that test work. I didn't dig into why, but you get a "type must be known" error.

nikomatsakis · 2016-04-14T13:59:46Z

oh, I see, because it calls structurally_resolved_type

Do not require the target type to be fully known, either. This allows code like `let x: *const () = 0 as _` to work (see regression test).

nikomatsakis · 2016-04-14T14:06:32Z

OK, @arielb1, pushed a new version that fixes the case you mentioned.

nikomatsakis · 2016-04-21T18:48:11Z

r? @pnkfelix (@arielb1 seems busy)

bors · 2016-04-22T15:20:26Z

💔 Test failed - auto-win-gnu-64-opt

arielb1 · 2016-04-22T15:41:53Z

@bors retry

What the **** is going with our windows bots?

bors · 2016-04-22T15:43:27Z

⚡ Previous build results for auto-linux-64-cargotest, auto-linux-64-opt-rustbuild, auto-linux-64-x-armsf, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-ios-opt, auto-win-msvc-64-opt-rustbuild are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-x-android-t, auto-linux-64-x-armhf, auto-linux-64-x-freebsd, auto-linux-64-x-netbsd, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir...

bors · 2016-04-22T15:46:51Z

💔 Test failed - auto-win-gnu-32-opt

alexcrichton · 2016-04-22T16:12:55Z

@bors: retry

On Fri, Apr 22, 2016 at 8:46 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-gnu-32-opt
http://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt/builds/3924
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#32258 (comment)

@alexcrichton

Suppress fallback and ambiguity errors If the infcx has observed other errors, then suppress both default type parameter fallback (which can be unreliable, as the full constraint set is not available) and errors related to unresovled variables (annoyingly, integer type variables cannot currently be unified with error, so that has to be a separate mechanism). Also add a flag to `infcx` to allow us to independently indicate when we have observed an error and hence should trigger this suppression mode. Fixesrust-lang#31997 cc @alexcrichton r? @arielb1

bors · 2016-04-22T23:40:25Z

⌛ Testing commit 89bbd2c with merge d43fb29...

bors · 2016-04-23T00:54:29Z

💔 Test failed - auto-win-msvc-32-opt

alexcrichton · 2016-04-23T01:04:54Z

@bors: retry

On Fri, Apr 22, 2016 at 5:54 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-msvc-32-opt
http://buildbot.rust-lang.org/builders/auto-win-msvc-32-opt/builds/3122
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#32258 (comment)

bors · 2016-04-23T03:27:21Z

⌛ Testing commit 89bbd2c with merge b428b62...

bors · 2016-04-23T04:47:53Z

💔 Test failed - auto-win-msvc-32-opt

alexcrichton · 2016-04-23T06:28:42Z

@bors: retry

On Fri, Apr 22, 2016 at 9:47 PM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-win-msvc-32-opt
http://buildbot.rust-lang.org/builders/auto-win-msvc-32-opt/builds/3124
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#32258 (comment)

bors · 2016-04-23T07:22:18Z

⌛ Testing commit 89bbd2c with merge 3a44be5...

bors · 2016-04-23T08:18:02Z

💔 Test failed - auto-mac-64-nopt-t

nikomatsakis · 2016-04-25T13:39:05Z

@bors r=arielb1

bors · 2016-04-25T13:39:05Z

📌 Commit b3d54a2 has been approved by arielb1

bors · 2016-04-25T15:13:22Z

⌛ Testing commit b3d54a2 with merge 90318b8...

@alexcrichton

Suppress fallback and ambiguity errors If the infcx has observed other errors, then suppress both default type parameter fallback (which can be unreliable, as the full constraint set is not available) and errors related to unresovled variables (annoyingly, integer type variables cannot currently be unified with error, so that has to be a separate mechanism). Also add a flag to `infcx` to allow us to independently indicate when we have observed an error and hence should trigger this suppression mode. Fixes#31997 cc @alexcrichton r? @arielb1

bors · 2016-04-25T17:43:34Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-32cross-opt, auto-linux-64-cargotest, auto-linux-64-cross-armhf, auto-linux-64-cross-armsf, auto-linux-64-cross-freebsd, auto-linux-64-cross-netbsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-cross-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild

nagisa · 2016-05-17T13:23:51Z

Tagging this as relnotes because of #33657

nikomatsakis assigned nikomatsakis and arielb1 and unassigned nikomatsakis Mar 15, 2016

arielb1 reviewed Mar 15, 2016
View reviewed changes

nikomatsakis force-pushed the fewer-errors branch 2 times, most recently from 1739d95 to f19c8c4Compare April 5, 2016 22:12

nikomatsakis mentioned this pull request Apr 5, 2016
Resolve failures cause too many typechecking failures #31997
Closed

nikomatsakis added 5 commits April 12, 2016 13:29

avoid "type must be known here" errors if tainted
a4e0e6b

move checking for unsized target type into cast
b023fcc
It is odd to have this logic strewn about. This also means that all calls to `type_is_known_to_be_sized` are encapsulated in the cast code, in case we want to update that logic.

move regr test into same directory
47d3b80

fix various error messages
2c9dfaf

nikomatsakis force-pushed the fewer-errors branch from 77c7754 to 2c9dfafCompare April 12, 2016 18:05

arielb1 reviewed Apr 14, 2016
View reviewed changes

Be a bit more constrained in our early check
89bbd2c
Do not require the target type to be fully known, either. This allows code like `let x: *const () = 0 as _` to work (see regression test).

nikomatsakis force-pushed the fewer-errors branch from 86a1274 to 89bbd2cCompare April 14, 2016 14:06

Manishearth mentioned this pull request Apr 22, 2016
Rollup of 6 pull requests #33154
Closed

patch test due to changes from compiletest-json
b3d54a2

bors merged commit b3d54a2 into rust-lang:masterApr 25, 2016

arielb1 mentioned this pull request May 17, 2016
Inference rules changed on 1.10 nightly #33657
Closed

nagisa added the relnotes Marks issues that should be documented in the release notes of the next release. label May 17, 2016

nikomatsakis deleted the fewer-errors branch October 3, 2016 14:55

Uh oh!

Uh oh!

Conversation

nikomatsakis commented Mar 15, 2016

Uh oh!

arielb1Mar 15, 2016

Choose a reason for hiding this comment

Uh oh!

nikomatsakisMar 15, 2016

Choose a reason for hiding this comment

Uh oh!

alexcrichton commented Mar 15, 2016

Uh oh!

nikomatsakis commented Mar 15, 2016

Uh oh!

nikomatsakis commented Apr 5, 2016

Uh oh!

arielb1 commented Apr 6, 2016

Uh oh!

arielb1 commented Apr 6, 2016

Uh oh!

nikomatsakis commented Apr 12, 2016

Uh oh!

arielb1 commented Apr 14, 2016

Uh oh!

arielb1 commented Apr 14, 2016

Uh oh!

arielb1Apr 14, 2016

Choose a reason for hiding this comment

Uh oh!

nikomatsakisApr 14, 2016

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Apr 14, 2016

Uh oh!

nikomatsakis commented Apr 14, 2016

Uh oh!

nikomatsakis commented Apr 14, 2016

Uh oh!

nikomatsakis commented Apr 21, 2016

Uh oh!

bors commented Apr 22, 2016

Uh oh!

arielb1 commented Apr 22, 2016

Uh oh!

bors commented Apr 22, 2016

Uh oh!

bors commented Apr 22, 2016

Uh oh!

alexcrichton commented Apr 22, 2016

Uh oh!

bors commented Apr 22, 2016

Uh oh!

bors commented Apr 23, 2016

Uh oh!

alexcrichton commented Apr 23, 2016

Uh oh!

bors commented Apr 23, 2016

Uh oh!

bors commented Apr 23, 2016

Uh oh!

alexcrichton commented Apr 23, 2016

Uh oh!

bors commented Apr 23, 2016

Uh oh!

bors commented Apr 23, 2016

Uh oh!

nikomatsakis commented Apr 25, 2016

Uh oh!

bors commented Apr 25, 2016

Uh oh!

bors commented Apr 25, 2016

Uh oh!

bors commented Apr 25, 2016

Uh oh!

nagisa commented May 17, 2016

Uh oh!

Reviewers

Assignees

Labels