Skip to content

Commit

Permalink
Rollup merge of #132214 - fmease:mv-impl-trait-val-paths, r=compiler-…
Browse files Browse the repository at this point in the history
…errors

Cleanup: Move an impl-Trait check from AST validation to AST lowering

Namely the one that rejects `impl Trait` in qself types and non-final path segments.
There's no good reason to perform this during AST validation.
We have better infrastructure in place in the AST lowerer (`ImplTraitContext`).
This shaves off a lot of code.
We now lower `impl Trait` in bad positions to `{type error}` which allows us to
remove a special case from HIR ty lowering.

Coincidentally fixes #126725. Well, it only *masks* it by passing `{type error}` to HIR analysis instead of a "bad" opaque. I was able to find a new reproducer for it. See the issue.
  • Loading branch information
matthiaskrgr authored Oct 27, 2024
2 parents 60beb59 + 442f395 commit 574a8ba
Show file tree
Hide file tree
Showing 13 changed files with 78 additions and 153 deletions.
19 changes: 16 additions & 3 deletions compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
modifiers: Option<ast::TraitBoundModifiers>,
) -> hir::QPath<'hir> {
let qself_position = qself.as_ref().map(|q| q.position);
let qself = qself.as_ref().map(|q| self.lower_ty(&q.ty, itctx));
let qself = qself
.as_ref()
// Reject cases like `<impl Trait>::Assoc` and `<impl Trait as Trait>::Assoc`.
.map(|q| self.lower_ty(&q.ty, ImplTraitContext::Disallowed(ImplTraitPosition::Path)));

let partial_res =
self.resolver.get_partial_res(id).unwrap_or_else(|| PartialRes::new(Res::Err));
Expand Down Expand Up @@ -75,6 +78,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
None
};

// Only permit `impl Trait` in the final segment. E.g., we permit `Option<impl Trait>`,
// `option::Option<T>::Xyz<impl Trait>` and reject `option::Option<impl Trait>::Xyz`.
let itctx = |i| {
if i + 1 == p.segments.len() {
itctx
} else {
ImplTraitContext::Disallowed(ImplTraitPosition::Path)
}
};

let path_span_lo = p.span.shrink_to_lo();
let proj_start = p.segments.len() - unresolved_segments;
let path = self.arena.alloc(hir::Path {
Expand Down Expand Up @@ -121,7 +134,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
segment,
param_mode,
generic_args_mode,
itctx,
itctx(i),
bound_modifier_allowed_features.clone(),
)
},
Expand Down Expand Up @@ -185,7 +198,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
segment,
param_mode,
generic_args_mode,
itctx,
itctx(i),
None,
));
let qpath = hir::QPath::TypeRelative(ty, hir_segment);
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_ast_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ ast_passes_generic_before_constraints = generic arguments must come before the f
ast_passes_generic_default_trailing = generic parameters with a default must be trailing
ast_passes_impl_trait_path = `impl Trait` is not allowed in path parameters
ast_passes_incompatible_features = `{$f1}` and `{$f2}` are incompatible, using them at the same time is not allowed
.help = remove one of these features
Expand Down
46 changes: 0 additions & 46 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,6 @@ struct AstValidator<'a> {

disallow_tilde_const: Option<TildeConstReason>,

/// Used to ban `impl Trait` in path projections like `<impl Iterator>::Item`
/// or `Foo::Bar<impl Trait>`
is_impl_trait_banned: bool,

/// Used to ban explicit safety on foreign items when the extern block is not marked as unsafe.
extern_mod_safety: Option<Safety>,

Expand Down Expand Up @@ -123,12 +119,6 @@ impl<'a> AstValidator<'a> {
self.extern_mod_safety = old;
}

fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
let old = mem::replace(&mut self.is_impl_trait_banned, true);
f(self);
self.is_impl_trait_banned = old;
}

fn with_tilde_const(
&mut self,
disallowed: Option<TildeConstReason>,
Expand Down Expand Up @@ -213,37 +203,6 @@ impl<'a> AstValidator<'a> {
.with_tilde_const(Some(TildeConstReason::TraitObject), |this| {
visit::walk_ty(this, t)
}),
TyKind::Path(qself, path) => {
// We allow these:
// - `Option<impl Trait>`
// - `option::Option<impl Trait>`
// - `option::Option<T>::Foo<impl Trait>`
//
// But not these:
// - `<impl Trait>::Foo`
// - `option::Option<impl Trait>::Foo`.
//
// To implement this, we disallow `impl Trait` from `qself`
// (for cases like `<impl Trait>::Foo>`)
// but we allow `impl Trait` in `GenericArgs`
// iff there are no more PathSegments.
if let Some(qself) = qself {
// `impl Trait` in `qself` is always illegal
self.with_banned_impl_trait(|this| this.visit_ty(&qself.ty));
}

// Note that there should be a call to visit_path here,
// so if any logic is added to process `Path`s a call to it should be
// added both in visit_path and here. This code mirrors visit::walk_path.
for (i, segment) in path.segments.iter().enumerate() {
// Allow `impl Trait` iff we're on the final path segment
if i == path.segments.len() - 1 {
self.visit_path_segment(segment);
} else {
self.with_banned_impl_trait(|this| this.visit_path_segment(segment));
}
}
}
_ => visit::walk_ty(self, t),
}
}
Expand Down Expand Up @@ -737,10 +696,6 @@ impl<'a> AstValidator<'a> {
}
}
TyKind::ImplTrait(_, bounds) => {
if self.is_impl_trait_banned {
self.dcx().emit_err(errors::ImplTraitPath { span: ty.span });
}

if let Some(outer_impl_trait_sp) = self.outer_impl_trait {
self.dcx().emit_err(errors::NestedImplTrait {
span: ty.span,
Expand Down Expand Up @@ -1729,7 +1684,6 @@ pub fn check_crate(
has_proc_macro_decls: false,
outer_impl_trait: None,
disallow_tilde_const: Some(TildeConstReason::Item),
is_impl_trait_banned: false,
extern_mod_safety: None,
lint_buffer: lints,
};
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,13 +418,6 @@ pub(crate) struct TraitObjectBound {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_impl_trait_path, code = E0667)]
pub(crate) struct ImplTraitPath {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(ast_passes_nested_impl_trait, code = E0666)]
pub(crate) struct NestedImplTrait {
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0667.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#### Note: this error code is no longer emitted by the compiler.

`impl Trait` is not allowed in path parameters.

Erroneous code example:

```compile_fail,E0667
```ignore (removed error code)
fn some_fn(mut x: impl Iterator) -> <impl Iterator>::Item { // error!
x.next().unwrap()
}
Expand All @@ -11,7 +13,7 @@ fn some_fn(mut x: impl Iterator) -> <impl Iterator>::Item { // error!
You cannot use `impl Trait` in path parameters. If you want something
equivalent, you can do this instead:

```
```ignore (removed error code)
fn some_fn<T: Iterator>(mut x: T) -> T::Item { // ok!
x.next().unwrap()
}
Expand Down
9 changes: 0 additions & 9 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1203,15 +1203,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
err.emit()
} else if let Err(reported) = qself_ty.error_reported() {
reported
} else if let ty::Alias(ty::Opaque, alias_ty) = qself_ty.kind() {
// `<impl Trait as OtherTrait>::Assoc` makes no sense.
struct_span_code_err!(
self.dcx(),
tcx.def_span(alias_ty.def_id),
E0667,
"`impl Trait` is not allowed in path parameters"
)
.emit() // Already reported in an earlier stage.
} else {
self.maybe_report_similar_assoc_fn(span, qself_ty, qself)?;

Expand Down
20 changes: 0 additions & 20 deletions tests/crashes/126725.rs

This file was deleted.

11 changes: 4 additions & 7 deletions tests/ui/impl-trait/impl_trait_projections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,27 @@ fn path_parametrized_type_is_allowed() -> option::Option<impl Debug> {
}

fn projection_is_disallowed(x: impl Iterator) -> <impl Iterator>::Item {
//~^ ERROR `impl Trait` is not allowed in path parameters
//~| ERROR `impl Trait` is not allowed in path parameters
//~^ ERROR `impl Trait` is not allowed in paths
x.next().unwrap()
}

fn projection_with_named_trait_is_disallowed(mut x: impl Iterator)
-> <impl Iterator as Iterator>::Item
//~^ ERROR `impl Trait` is not allowed in path parameters
//~^ ERROR `impl Trait` is not allowed in paths
{
x.next().unwrap()
}

fn projection_with_named_trait_inside_path_is_disallowed()
-> <::std::ops::Range<impl Debug> as Iterator>::Item
//~^ ERROR `impl Trait` is not allowed in path parameters
//~| ERROR `impl Debug: Step` is not satisfied
//~^ ERROR `impl Trait` is not allowed in paths
{
//~^ ERROR `impl Debug: Step` is not satisfied
(1i32..100).next().unwrap()
}

fn projection_from_impl_trait_inside_dyn_trait_is_disallowed()
-> <dyn Iterator<Item = impl Debug> as Iterator>::Item
//~^ ERROR `impl Trait` is not allowed in path parameters
//~^ ERROR `impl Trait` is not allowed in paths
{
panic!()
}
Expand Down
70 changes: 16 additions & 54 deletions tests/ui/impl-trait/impl_trait_projections.stderr
Original file line number Diff line number Diff line change
@@ -1,73 +1,35 @@
error[E0667]: `impl Trait` is not allowed in path parameters
error[E0562]: `impl Trait` is not allowed in paths
--> $DIR/impl_trait_projections.rs:12:51
|
LL | fn projection_is_disallowed(x: impl Iterator) -> <impl Iterator>::Item {
| ^^^^^^^^^^^^^
|
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error[E0667]: `impl Trait` is not allowed in path parameters
--> $DIR/impl_trait_projections.rs:19:9
error[E0562]: `impl Trait` is not allowed in paths
--> $DIR/impl_trait_projections.rs:18:9
|
LL | -> <impl Iterator as Iterator>::Item
| ^^^^^^^^^^^^^
|
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error[E0667]: `impl Trait` is not allowed in path parameters
--> $DIR/impl_trait_projections.rs:26:27
error[E0562]: `impl Trait` is not allowed in paths
--> $DIR/impl_trait_projections.rs:25:27
|
LL | -> <::std::ops::Range<impl Debug> as Iterator>::Item
| ^^^^^^^^^^
|
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error[E0667]: `impl Trait` is not allowed in path parameters
--> $DIR/impl_trait_projections.rs:35:29
error[E0562]: `impl Trait` is not allowed in paths
--> $DIR/impl_trait_projections.rs:32:29
|
LL | -> <dyn Iterator<Item = impl Debug> as Iterator>::Item
| ^^^^^^^^^^

error[E0667]: `impl Trait` is not allowed in path parameters
--> $DIR/impl_trait_projections.rs:12:51
|
LL | fn projection_is_disallowed(x: impl Iterator) -> <impl Iterator>::Item {
| ^^^^^^^^^^^^^

error[E0277]: the trait bound `impl Debug: Step` is not satisfied
--> $DIR/impl_trait_projections.rs:26:8
|
LL | -> <::std::ops::Range<impl Debug> as Iterator>::Item
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Step` is not implemented for `impl Debug`, which is required by `std::ops::Range<impl Debug>: Iterator`
|
= help: the following other types implement trait `Step`:
Char
Ipv4Addr
Ipv6Addr
char
i128
i16
i32
i64
and 8 others
= note: required for `std::ops::Range<impl Debug>` to implement `Iterator`

error[E0277]: the trait bound `impl Debug: Step` is not satisfied
--> $DIR/impl_trait_projections.rs:29:1
|
LL | / {
LL | |
LL | | (1i32..100).next().unwrap()
LL | | }
| |_^ the trait `Step` is not implemented for `impl Debug`, which is required by `std::ops::Range<impl Debug>: Iterator`
|
= help: the following other types implement trait `Step`:
Char
Ipv4Addr
Ipv6Addr
char
i128
i16
i32
i64
and 8 others
= note: required for `std::ops::Range<impl Debug>` to implement `Iterator`
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error: aborting due to 7 previous errors
error: aborting due to 4 previous errors

Some errors have detailed explanations: E0277, E0667.
For more information about an error, try `rustc --explain E0277`.
For more information about this error, try `rustc --explain E0562`.
22 changes: 22 additions & 0 deletions tests/ui/impl-trait/in-trait/bad-projection-from-opaque.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// issue: rust-lang/rust#126725

trait Foo {
fn foo<'a>() -> <&'a impl Sized as Bar>::Output;
//~^ ERROR `impl Trait` is not allowed in paths
}

trait Bar {
type Output;
}

impl<'a> Bar for &'a () {
type Output = &'a i32;
}

impl Foo for () {
fn foo<'a>() -> <&'a Self as Bar>::Output {
&0
}
}

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/impl-trait/in-trait/bad-projection-from-opaque.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0562]: `impl Trait` is not allowed in paths
--> $DIR/bad-projection-from-opaque.rs:4:26
|
LL | fn foo<'a>() -> <&'a impl Sized as Bar>::Output;
| ^^^^^^^^^^
|
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0562`.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
pub trait Bar { }
pub trait Quux<T> { type Assoc; }
pub fn demo(_: impl Quux<(), Assoc=<() as Quux<impl Bar>>::Assoc>) { }
//~^ ERROR `impl Trait` is not allowed in path parameters
//~^ ERROR `impl Trait` is not allowed in paths
impl<T> Quux<T> for () { type Assoc = u32; }

fn main() { }
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
error[E0667]: `impl Trait` is not allowed in path parameters
error[E0562]: `impl Trait` is not allowed in paths
--> $DIR/issue-57979-impl-trait-in-path.rs:8:48
|
LL | pub fn demo(_: impl Quux<(), Assoc=<() as Quux<impl Bar>>::Assoc>) { }
| ^^^^^^^^
|
= note: `impl Trait` is only allowed in arguments and return types of functions and methods

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0667`.
For more information about this error, try `rustc --explain E0562`.

0 comments on commit 574a8ba

Please sign in to comment.