[WIP] rustc_typeck: check well-formedness of type aliases. #54033

eddyb · 2018-09-07T15:42:56Z

Enforce that for type aliases (type Foo<T: Trait> = Bar<T>;), all the bounds required by the type being aliased (Bar<T>) are satisfied (e.g. by T: Trait).

On Rust 2015, this is reported as a lint, but on Rust 2018 it's a hard error.
(see #49441 (comment) for more details on the decision taken)

NOTE: this is only the first half, we also need to check declared bounds assuming the RHS type is WF (to ensure the type alias is fully equivalent to its RHS type, everywhere it's used).
EDIT: second half is up at #54090.

Fixes #44075, fixes #51626.

r? @nikomatsakis

Add bounds to type aliases (needed by rust-lang/rust#54033).

eddyb · 2018-09-07T22:10:10Z

@bors try (for crater in check mode)

bors · 2018-09-07T22:10:21Z

⌛ Trying commit 013d7f0634496b2d2defd8acce43fcbed1163526 with merge 0ed22d1874786c4c6a348e14674cad1dc19df6a0...

bors · 2018-09-08T00:12:19Z

💔 Test failed - status-travis

eddyb · 2018-09-08T06:38:45Z

Okay, so serde is already a problem there, which means we can't easily do the crater run I wanted to do. I'll move to warn on Rust 2015 and only error on Rust 2018.

Centril · 2018-09-08T07:35:33Z

@eddyb yeah that sounds reasonable.

eddyb · 2018-09-17T00:20:04Z

@bors try

bors · 2018-09-17T00:20:23Z

⌛ Trying commit 0568e27db5f862d27f210c0dac637870a4c209de with merge ecd082c0984889e4676a3dd794753309c854ab46...

Mark-Simulacrum · 2018-09-17T00:23:39Z

@craterbot command follows: run start=master#cb6d2dfa8923902b0992a1522dc4a45a9d3ba690 end=try#cfeeefe83c83bceb55d60d7e2be173db7597ed50 mode=check-only

eddyb · 2018-09-17T08:32:15Z

(just pushed test fixes only, try build shouldn't care)

bors · 2018-09-17T09:00:15Z

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

eddyb · 2018-09-17T10:32:22Z

@craterbot run start=master#cb6d2dfa8923902b0992a1522dc4a45a9d3ba690 end=try#cfeeefe83c83bceb55d60d7e2be173db7597ed50 mode=check-only

craterbot · 2018-09-17T10:32:23Z

👌 Experiment pr-54033 created and queued.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2018-09-17T10:32:24Z

🚧 Experiment pr-54033 is now running on agent aws-1.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

nikomatsakis · 2018-09-17T15:46:15Z

src/librustc/infer/error_reporting/need_type_info.rs

+            let local_id_root = tables.local_id_root?;
+            assert!(local_id_root.is_local());
+
+            tables.node_types().iter().filter_map(|(&local_id, &node_ty)| {


Nit: It'd be nice to pull this out into some sort of helper function, I think? I'd also like to see a comment, something like:

Iterate over the all the things that we've assigned types to. Check whether they represent local variables and, if so, if the type of that local variable references the type we are looking for.

nikomatsakis · 2018-09-17T15:49:46Z

src/librustc/traits/error_reporting.rs

+
+            let span = obligation.cause.span;
+            let lint = self.get_lint_from_cause_code(&obligation.cause.code);
+            macro_rules! struct_span_err_or_lint {


Nit: I feel like I've seen this macro at least twice. Can't we abstract this somehow? Or move it?

It specifically takes advantage of variables in scope, so sadly it wouldn't be that easy.
We'd have to take both infcx and obligation.cause (instead of tcx.sess and span), I think? Maybe it is easy.

What do you think struct_obligation_err_or_lint, as a name for the macro?
Also, is it just me or is struct just not the best word to use?
I almost want to move everything to diagnostic builders so we can get rid of the non-struct_ versions and remove the struct_ prefix.

nikomatsakis · 2018-09-17T15:50:19Z

src/librustc/traits/error_reporting.rs

    pub fn report_selection_error(&self,
                                  obligation: &PredicateObligation<'tcx>,
                                  error: &SelectionError<'tcx>,
                                  fallback_has_occurred: bool)
    {
        let span = obligation.cause.span;

+        let lint = self.get_lint_from_cause_code(&obligation.cause.code);
+        macro_rules! struct_span_err_or_lint {


This makes 3 times :)

nikomatsakis · 2018-09-17T15:57:23Z

src/librustc/traits/error_reporting.rs

@@ -50,7 +50,6 @@ use syntax_pos::{DUMMY_SP, Span};
 impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
    pub fn report_fulfillment_errors(&self,
                                     errors: &[FulfillmentError<'tcx>],
-                                     body_id: Option<hir::BodyId>,


I take it we're getting the body_id from the ObligationCause here? I had planned to remove that field once we transition to NLL, since it won't be needed.

But I guess we could thread this info back through then.

This was only used for the code in need_type_info.rs that was visiting the HIR so thankfully I think we won't need it!

I actually couldn't visit the HIR because the body_id on the obligation was sometimes "not large enough" to include the closure that had the offending argument.

craterbot · 2018-09-18T04:30:38Z

🎉 Experiment pr-54033 is completed!
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

eddyb · 2018-09-18T05:25:35Z

See #49441 (comment) - TL;DR we can't make "not enough bounds" an error in Rust 2018.
However, "too many bounds" (currently in #54090) is definitely doable, so I'll focus on that.

eddyb · 2018-09-20T09:47:19Z

src/librustc_typeck/check/wfcheck.rs

-                                                                fcx.body_id,
-                                                                p,
-                                                                span));
+                                                                &cause,


This is fine, but fcx.normalize_associated_types_in(span, &predicates) above should be fcx.inh.normalize_associated_types_in(cause, fcx.param_env, &predicates) instead.

TimNN · 2018-10-16T17:41:03Z

Ping from triage @eddyb: What is the status of this PR?

nikomatsakis · 2018-10-24T20:08:05Z

I'm going to close this PR for now. I do plan to start opening some PRs and issuing warnings, per the plan discussed in #55222

rust-highfive assigned nikomatsakis Sep 7, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 7, 2018

This comment has been minimized.

Sign in to view

eddyb force-pushed the alias-waffles branch 2 times, most recently from 4c9ffde to a3f11cf Compare September 7, 2018 16:47

This comment has been minimized.

Sign in to view

eddyb added a commit to eddyb/ena that referenced this pull request Sep 7, 2018

Add bounds to type aliases (needed by rust-lang/rust#54033).

fec1286

nikomatsakis added a commit to rust-lang/ena that referenced this pull request Sep 7, 2018

Merge pull request #13 from eddyb/alias-waffles

31c5efb

Add bounds to type aliases (needed by rust-lang/rust#54033).

eddyb force-pushed the alias-waffles branch from a3f11cf to 013d7f0 Compare September 7, 2018 22:09

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 8, 2018

This comment has been minimized.

Sign in to view

This was referenced Sep 9, 2018

Reject bounds in type aliases with edition 2024 #49441

Open

[WIP] rustc_typeck: ensure type alias bounds are implied by the type being well-formed. #54090

Closed

eddyb force-pushed the alias-waffles branch from 013d7f0 to 0568e27 Compare September 17, 2018 00:14

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 17, 2018

nikomatsakis approved these changes Sep 17, 2018

View reviewed changes

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Sep 18, 2018

eddyb mentioned this pull request Sep 19, 2018

[WIP] rustc: check that type alias where clauses are well-formed. #54352

Closed

eddyb commented Sep 20, 2018

View reviewed changes

eddyb mentioned this pull request Sep 20, 2018

[WIP] rustc_typeck: make "global" WF requirements hard errors, in type aliases. #54394

Closed

TimNN added A-allocators Area: Custom and system allocators and removed A-allocators Area: Custom and system allocators labels Sep 25, 2018

TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2018

Centril added P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels Oct 19, 2018

Centril mentioned this pull request Oct 20, 2018

finish up type alias PRs #55222

Closed

nikomatsakis closed this Oct 24, 2018

apiraino removed P-high High priority I-nominated labels Feb 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] rustc_typeck: check well-formedness of type aliases. #54033

[WIP] rustc_typeck: check well-formedness of type aliases. #54033

eddyb commented Sep 7, 2018 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Sep 7, 2018

bors commented Sep 7, 2018

This comment has been minimized.

bors commented Sep 8, 2018

This comment has been minimized.

eddyb commented Sep 8, 2018

Centril commented Sep 8, 2018

eddyb commented Sep 17, 2018

bors commented Sep 17, 2018

Mark-Simulacrum commented Sep 17, 2018 •

edited by eddyb

Loading

eddyb commented Sep 17, 2018

bors commented Sep 17, 2018

eddyb commented Sep 17, 2018

craterbot commented Sep 17, 2018

craterbot commented Sep 17, 2018

nikomatsakis Sep 17, 2018

nikomatsakis Sep 17, 2018

nikomatsakis Sep 17, 2018

eddyb Sep 17, 2018

eddyb Sep 17, 2018

nikomatsakis Sep 17, 2018

nikomatsakis Sep 17, 2018

nikomatsakis Sep 17, 2018

eddyb Sep 17, 2018

craterbot commented Sep 18, 2018

eddyb commented Sep 18, 2018

eddyb Sep 20, 2018

TimNN commented Oct 16, 2018

nikomatsakis commented Oct 24, 2018

[WIP] rustc_typeck: check well-formedness of type aliases. #54033

[WIP] rustc_typeck: check well-formedness of type aliases. #54033

Conversation

eddyb commented Sep 7, 2018 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Sep 7, 2018

bors commented Sep 7, 2018

This comment has been minimized.

bors commented Sep 8, 2018

This comment has been minimized.

eddyb commented Sep 8, 2018

Centril commented Sep 8, 2018

eddyb commented Sep 17, 2018

bors commented Sep 17, 2018

Mark-Simulacrum commented Sep 17, 2018 • edited by eddyb Loading

eddyb commented Sep 17, 2018

bors commented Sep 17, 2018

eddyb commented Sep 17, 2018

craterbot commented Sep 17, 2018

craterbot commented Sep 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

craterbot commented Sep 18, 2018

eddyb commented Sep 18, 2018

Choose a reason for hiding this comment

TimNN commented Oct 16, 2018

nikomatsakis commented Oct 24, 2018

eddyb commented Sep 7, 2018 •

edited

Loading

Mark-Simulacrum commented Sep 17, 2018 •

edited by eddyb

Loading