Handle associated types in `const` context #70056

estebank · 2020-03-16T20:27:35Z

When in const context (like array lengths) we erase all substs.
Because of this, we used to emit spurious suggestions to restrict type
parameters when using associated types on them, even though that would
never work. Instead, we now signal const contexts when evaluating
traits and emit a more accurate error.

Address the diagnostics part of #43408 and #61730.

When in `const` context (like array lengths) we erase all substs. Because of this, we used to emit spurious suggestions to restrict type parameters when using associated types on them, even though that would never work. Instead, we now signal `const` contexts when evaluating traits and emit a more accurate error.

rust-highfive · 2020-03-16T20:27:38Z

r? @davidtwco

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

estebank · 2020-03-16T20:28:35Z

This is a bit of a blunt approach and we should verify this is a reasonable way to do this.

CC @rust-lang/compiler @varkor @oli-obk

src/librustc_typeck/check/mod.rs

src/test/ui/inference/inference_unstable.rs

src/librustc_trait_selection/traits/error_reporting/suggestions.rs

src/librustc_typeck/check/mod.rs

eddyb · 2020-03-16T22:31:38Z

@estebank This seems overkill when lazy normalization is on the horizon.

eddyb · 2020-03-16T22:34:11Z

src/librustc_trait_selection/traits/error_reporting/suggestions.rs

@@ -158,6 +158,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
            ty::Projection(projection) => (false, Some(projection)),
            _ => return,
        };
+        if self.in_const_context.get() {


This is weirdly specific given that anything involving type parameters doesn't work.

This is a very common thing people encounter when trying to be too clever by half. I'm looking for other cases that are likely to be hit so that I can also handle them. One case I'm looking at now is the following, which ICEs

fn test<T: ?Sized>() { [0u8; std::mem::size_of::<&T>()]; }

We should fix generics_of if the parent of AnonConst is an Expr (and probably any Ty nested in a body, but that's harder to check), I think I've suggested that before. It should just work, the query cycles only come from types outside bodies.

And by "fix" I mean a trivial whitelist, much smaller than this PR.

estebank · 2020-03-16T23:29:32Z

This seems overkill when lazy normalization is on the horizon.

I understand that, but having a stop gap in place until then is in my eyes worthwhile, given how often people hit this limitation with a very misleading error. If lazy normalization won't be hitting stable in the next couple of releases (which I don't think will happen), then we need something avoid sowing further confusion. I would be more than happy to accomplish the same with an alternative approach if one is workable.

estebank · 2020-03-17T02:25:52Z

After reassessing the approach the second commit makes the change much smaller and way more reasonable.

src/test/ui/discrim/discrim-ill-typed.stderr

src/test/ui/repeat_count.stderr

eddyb · 2020-03-17T09:03:34Z

@estebank Hmm, if you want a "let's unconfuse users" stopgap, maybe we can do this instead: in astconv, whereever DefKind::TyParam is converted into ty::Param, we could emit a warning if it's not in tcx.generics_of(self.item_def_id) (you probably need a way to get the generics in scope from AstConv and they could also be cached in the implementers of AstConv).

(thankfully ty::ConstKind::Param doesn't need this as #![feature(const_generics)] doesn't ever use the wrong generics, at the cost of breaking a few things pre-lazy-normalization)

Note that I'm suggesting a warning and not an error. We don't want to break anything that already compiles, however useless it may be.
The warning message could say something along the lines of "this type parameter is not yet correctly supported here, misleading errors may occur".

eddyb · 2020-03-17T09:04:58Z

src/librustc/hir/map/mod.rs

-            | Node::AnonConst(_)
-            | Node::Item(&Item { kind: ItemKind::Static(..), .. }) => true,
-            Node::Item(&Item { kind: ItemKind::Fn(ref sig, ..), .. }) => {
+        match self.find(parent_id) {


This is weird, why could find ever return None? We should fix that if it is the case (cc @Zoxc)

This was encountered on enum variants like enum E { A = 1f64 }, but I didn't dig too deep.

src/librustc/hir/map/mod.rs

davidtwco · 2020-03-18T09:59:58Z

r? @eddyb

estebank · 2020-03-26T02:02:22Z

@eddyb I think the current diff should be palatable to you as a stopgap until lazy eval.

eddyb · 2020-03-26T07:28:39Z

I still would prefer doing this: #70056 (comment).

eddyb · 2020-03-27T00:55:39Z

I've opened #70452 for my whitelist approach.

bors · 2020-03-30T02:13:35Z

☔ The latest upstream changes (presumably #70009) made this pull request unmergeable. Please resolve the merge conflicts.

rust-highfive assigned davidtwco Mar 16, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 16, 2020

oli-obk reviewed Mar 16, 2020

View reviewed changes

src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved

cramertj reviewed Mar 16, 2020

View reviewed changes

src/test/ui/inference/inference_unstable.rs Outdated Show resolved Hide resolved

Centril reviewed Mar 16, 2020

View reviewed changes

src/librustc_trait_selection/traits/error_reporting/suggestions.rs Outdated Show resolved Hide resolved

Centril reviewed Mar 16, 2020

View reviewed changes

src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved

eddyb reviewed Mar 16, 2020

View reviewed changes

review comments: avoid changing the queries

9a821e8

Centril reviewed Mar 17, 2020

View reviewed changes

src/test/ui/discrim/discrim-ill-typed.stderr Outdated Show resolved Hide resolved

Centril reviewed Mar 17, 2020

View reviewed changes

src/test/ui/repeat_count.stderr Show resolved Hide resolved

review comment: fix wording

f00ead1

eddyb reviewed Mar 17, 2020

View reviewed changes

src/librustc/hir/map/mod.rs Outdated Show resolved Hide resolved

estebank added 2 commits March 17, 2020 09:53

Rework is_const_context to not rely on get_parent_item

84a5706

Fix check_for_cast

f02aeae

rust-highfive assigned eddyb and unassigned davidtwco Mar 18, 2020

JohnCSimon removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2020

JohnCSimon added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 6, 2020

estebank closed this Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle associated types in `const` context #70056

Handle associated types in `const` context #70056

estebank commented Mar 16, 2020

rust-highfive commented Mar 16, 2020

estebank commented Mar 16, 2020 •

edited

Loading

eddyb commented Mar 16, 2020

eddyb Mar 16, 2020

estebank Mar 16, 2020 •

edited

Loading

eddyb Mar 17, 2020

eddyb Mar 17, 2020

estebank Mar 27, 2020

estebank commented Mar 16, 2020

estebank commented Mar 17, 2020

eddyb commented Mar 17, 2020

eddyb Mar 17, 2020

estebank Mar 17, 2020

davidtwco commented Mar 18, 2020

estebank commented Mar 26, 2020

eddyb commented Mar 26, 2020

eddyb commented Mar 27, 2020

bors commented Mar 30, 2020

Handle associated types in const context #70056

Handle associated types in const context #70056

Conversation

estebank commented Mar 16, 2020

rust-highfive commented Mar 16, 2020

estebank commented Mar 16, 2020 • edited Loading

eddyb commented Mar 16, 2020

eddyb Mar 16, 2020

Choose a reason for hiding this comment

estebank Mar 16, 2020 • edited Loading

Choose a reason for hiding this comment

eddyb Mar 17, 2020

Choose a reason for hiding this comment

eddyb Mar 17, 2020

Choose a reason for hiding this comment

estebank Mar 27, 2020

Choose a reason for hiding this comment

estebank commented Mar 16, 2020

estebank commented Mar 17, 2020

eddyb commented Mar 17, 2020

eddyb Mar 17, 2020

Choose a reason for hiding this comment

estebank Mar 17, 2020

Choose a reason for hiding this comment

davidtwco commented Mar 18, 2020

estebank commented Mar 26, 2020

eddyb commented Mar 26, 2020

eddyb commented Mar 27, 2020

bors commented Mar 30, 2020

Handle associated types in `const` context #70056

Handle associated types in `const` context #70056

estebank commented Mar 16, 2020 •

edited

Loading

estebank Mar 16, 2020 •

edited

Loading