Add a separate error for `dyn Trait` in `const fn` #89021

WaffleLapkin · 2021-09-16T18:36:52Z

Previously "trait bounds other than Sized on const fn parameters are unstable" error was used for both trait bounds (<T: Trait>) and trait objects (dyn Trait). This was pretty confusing.

This PR adds a separate error for trait objects: "trait objects in const fn are unstable". The error for trait bounds is otherwise intact.

This is follow up to #88907

r? @estebank

@rustbot label: +A-diagnostics

Previously "trait bounds other than `Sized` on const fn parameters are unstable" error was used for both trait bounds (<T: Trait>) and trait objects (dyn Trait). This was pretty confusing. This patch adds a separeta error for trait objects: "trait objects in const fn are unstable". The error for trait bounds is otherwise intact.

compiler/rustc_const_eval/src/transform/check_consts/check.rs

estebank · 2021-09-17T08:45:54Z

src/test/ui/consts/min_const_fn/min_const_fn_dyn.rs

@@ -7,9 +7,9 @@ struct Hide(HasDyn);
 const fn no_inner_dyn_trait(_x: Hide) {}
 const fn no_inner_dyn_trait2(x: Hide) {
    x.0.field;
-//~^ ERROR trait bounds other than `Sized`
+//~^ ERROR trait objects in const fn are unstable


For the cases of fields like this one it would be nice to have a span_note pointing at the type and field's type to show how it is a trait object.

It would be really nice! Can we implement this though? It seems like we only check locals and not expressions:

rust/compiler/rustc_const_eval/src/transform/check_consts/check.rs

Lines 245 to 259 in f84000d

for (idx, local) in body.local_decls.iter_enumerated() {

// Handle the return place below.

if idx == RETURN_PLACE || local.internal {

continue;

}

self.span = local.source_info.span;

self.check_local_or_return_ty(local.ty, idx);

}

// impl trait is gone in MIR, so check the return type of a const fn by its signature

// instead of the type of the return place.

self.span = body.local_decls[RETURN_PLACE].source_info.span;

let return_ty = tcx.fn_sig(def_id).output();

self.check_local_or_return_ty(return_ty.skip_binder(), RETURN_PLACE);

I'm not sure if we can't get that info from local.local_info or local.source_info (I'm not that familiar with const eval and have to relearn it every time I look at it).

Hm, after reading docs I can't see how we can get the expression, but maybe I'm missing something

Can you check what the contents of local.local_info are? I assume it is going to be a VarBindingForm. If it is, what are the contents of opt_match_place?

Alternatively, if the data isn't there, it could potentially be added.

So in the case of our interest (field access) local.local_info is simply None :(

In that case, we'll have to make more dramatic changes so that we can track that data :-/

It's probably not high priority, but if you desire to take that on, I'd be more than happy to see it through!

I don't think I'm interested enough in that. Thanks for offering help though!

compiler/rustc_const_eval/src/transform/check_consts/check.rs

estebank · 2021-09-17T08:51:04Z

@bors r+

bors · 2021-09-17T08:51:06Z

📌 Commit f84000d has been approved by estebank

…_trait_in_const_fn, r=estebank Add a separate error for `dyn Trait` in `const fn` Previously "trait bounds other than `Sized` on const fn parameters are unstable" error was used for both trait bounds (`<T: Trait>`) and trait objects (`dyn Trait`). This was pretty confusing. This PR adds a separate error for trait objects: "trait objects in const fn are unstable". The error for trait bounds is otherwise intact. This is follow up to rust-lang#88907 r? `@estebank` `@rustbot` label: +A-diagnostics

Rollup of 10 pull requests Successful merges: - rust-lang#87960 (Suggest replacing an inexisting field for an unmentioned field) - rust-lang#88855 (Allow simd_shuffle to accept vectors of any length) - rust-lang#88966 (Check for shadowing issues involving block labels) - rust-lang#88996 (Fix linting when trailing macro expands to a trailing semi) - rust-lang#89017 (fix potential race in AtomicU64 time monotonizer) - rust-lang#89021 (Add a separate error for `dyn Trait` in `const fn`) - rust-lang#89051 (Add intra-doc links and small changes to `std::os` to be more consistent) - rust-lang#89053 (refactor: VecDeques IntoIter fields to private) - rust-lang#89055 (Suggest better place to add call parentheses for method expressions wrapped in parentheses) - rust-lang#89081 (Fix a typo) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rustbot added the A-diagnostics Area: Messages for errors, warnings, and lints label Sep 16, 2021

rust-highfive assigned estebank Sep 16, 2021

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

WaffleLapkin commented Sep 16, 2021

View reviewed changes

compiler/rustc_const_eval/src/transform/check_consts/check.rs Show resolved Hide resolved

estebank approved these changes Sep 17, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2021

JohnTitor mentioned this pull request Sep 19, 2021

Rollup of 11 pull requests #89087

Closed

JohnTitor mentioned this pull request Sep 19, 2021

Rollup of 10 pull requests #89089

Merged

bors merged commit 61bfe36 into rust-lang:master Sep 19, 2021

rustbot added this to the 1.57.0 milestone Sep 19, 2021

WaffleLapkin deleted the separate_error_for_dyn_trait_in_const_fn branch September 19, 2021 22:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a separate error for `dyn Trait` in `const fn` #89021

Add a separate error for `dyn Trait` in `const fn` #89021

WaffleLapkin commented Sep 16, 2021

estebank Sep 17, 2021

WaffleLapkin Sep 17, 2021

estebank Sep 17, 2021

WaffleLapkin Sep 18, 2021

estebank Sep 18, 2021 •

edited

Loading

WaffleLapkin Sep 20, 2021

estebank Sep 20, 2021

WaffleLapkin Sep 20, 2021

estebank commented Sep 17, 2021

bors commented Sep 17, 2021

	for (idx, local) in body.local_decls.iter_enumerated() {
	// Handle the return place below.
	if idx == RETURN_PLACE \|\| local.internal {
	continue;
	}

	self.span = local.source_info.span;
	self.check_local_or_return_ty(local.ty, idx);
	}

	// impl trait is gone in MIR, so check the return type of a const fn by its signature
	// instead of the type of the return place.
	self.span = body.local_decls[RETURN_PLACE].source_info.span;
	let return_ty = tcx.fn_sig(def_id).output();
	self.check_local_or_return_ty(return_ty.skip_binder(), RETURN_PLACE);

Add a separate error for dyn Trait in const fn #89021

Add a separate error for dyn Trait in const fn #89021

Conversation

WaffleLapkin commented Sep 16, 2021

estebank Sep 17, 2021

Choose a reason for hiding this comment

WaffleLapkin Sep 17, 2021

Choose a reason for hiding this comment

estebank Sep 17, 2021

Choose a reason for hiding this comment

WaffleLapkin Sep 18, 2021

Choose a reason for hiding this comment

estebank Sep 18, 2021 • edited Loading

Choose a reason for hiding this comment

WaffleLapkin Sep 20, 2021

Choose a reason for hiding this comment

estebank Sep 20, 2021

Choose a reason for hiding this comment

WaffleLapkin Sep 20, 2021

Choose a reason for hiding this comment

estebank commented Sep 17, 2021

bors commented Sep 17, 2021

Add a separate error for `dyn Trait` in `const fn` #89021

Add a separate error for `dyn Trait` in `const fn` #89021

estebank Sep 18, 2021 •

edited

Loading