From 8e30e15eaf5922f2e528f09ecaf2917a2ee2b87d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Apr 2020 13:54:09 -0700 Subject: [PATCH 1/3] Test for const expression missing braces --- .../const-expression-missing-braces.rs | 26 ++++++++++ .../const-expression-missing-braces.stderr | 48 +++++++++++++++++++ 2 files changed, 74 insertions(+) create mode 100644 src/test/ui/const-generics/const-expression-missing-braces.rs create mode 100644 src/test/ui/const-generics/const-expression-missing-braces.stderr diff --git a/src/test/ui/const-generics/const-expression-missing-braces.rs b/src/test/ui/const-generics/const-expression-missing-braces.rs new file mode 100644 index 0000000000000..d9a7054f6c4a4 --- /dev/null +++ b/src/test/ui/const-generics/const-expression-missing-braces.rs @@ -0,0 +1,26 @@ +#![allow(incomplete_features)] +#![feature(const_generics)] + +fn foo() {} + +fn a() { + let bar = 3; + foo::(); + //~^ ERROR expected one of `!`, `(`, `,`, `>`, `?`, `for`, lifetime, or path, found `3` +} +fn b() { + let bar = 3; + foo::(); + //~^ ERROR expected trait, found local variable `bar` + //~| ERROR expected trait, found local variable `bar` + //~| ERROR wrong number of const arguments: expected 1, found 0 + //~| ERROR wrong number of type arguments: expected 0, found 1 + //~| WARNING trait objects without an explicit `dyn` are deprecated +} +fn c() { + let bar = 3; + foo::<3 + 3>(); + //~^ ERROR expected one of `,` or `>`, found `+` +} + +fn main() {} diff --git a/src/test/ui/const-generics/const-expression-missing-braces.stderr b/src/test/ui/const-generics/const-expression-missing-braces.stderr new file mode 100644 index 0000000000000..3e03053f21ed8 --- /dev/null +++ b/src/test/ui/const-generics/const-expression-missing-braces.stderr @@ -0,0 +1,48 @@ +error: expected one of `!`, `(`, `,`, `>`, `?`, `for`, lifetime, or path, found `3` + --> $DIR/const-expression-missing-braces.rs:8:17 + | +LL | foo::(); + | ^ expected one of 8 possible tokens + +error: expected one of `,` or `>`, found `+` + --> $DIR/const-expression-missing-braces.rs:22:13 + | +LL | foo::<3 + 3>(); + | ^ expected one of `,` or `>` + +error[E0404]: expected trait, found local variable `bar` + --> $DIR/const-expression-missing-braces.rs:13:11 + | +LL | foo::(); + | ^^^ not a trait + +error[E0404]: expected trait, found local variable `bar` + --> $DIR/const-expression-missing-braces.rs:13:17 + | +LL | foo::(); + | ^^^ not a trait + +warning: trait objects without an explicit `dyn` are deprecated + --> $DIR/const-expression-missing-braces.rs:13:11 + | +LL | foo::(); + | ^^^^^^^^^ help: use `dyn`: `dyn bar + bar` + | + = note: `#[warn(bare_trait_objects)]` on by default + +error[E0107]: wrong number of const arguments: expected 1, found 0 + --> $DIR/const-expression-missing-braces.rs:13:5 + | +LL | foo::(); + | ^^^^^^^^^^^^^^^^ expected 1 const argument + +error[E0107]: wrong number of type arguments: expected 0, found 1 + --> $DIR/const-expression-missing-braces.rs:13:11 + | +LL | foo::(); + | ^^^^^^^^^ unexpected type argument + +error: aborting due to 6 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0107, E0404. +For more information about an error, try `rustc --explain E0107`. From eb4ba918e0774a51c316d2180092698e04a62736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 Apr 2020 14:16:53 -0700 Subject: [PATCH 2/3] Custom error for const expression parsed as trait object --- src/librustc_ast_lowering/lib.rs | 67 ++++++++++++++++++- src/librustc_middle/ty/sty.rs | 1 + src/librustc_resolve/late.rs | 67 +++++++++++++------ .../const-expression-missing-braces.rs | 6 +- .../const-expression-missing-braces.stderr | 37 ++-------- 5 files changed, 123 insertions(+), 55 deletions(-) diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 2cf81af04166c..b65aa817f830b 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -49,7 +49,7 @@ use rustc_ast_pretty::pprust; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; -use rustc_errors::struct_span_err; +use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Namespace, PartialRes, PerNS, Res}; use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId, CRATE_DEF_INDEX}; @@ -1136,6 +1136,71 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } } + // Possible `a + b` expression that should be surrounded in braces but was parsed + // as trait bounds in a trait object. Suggest surrounding with braces. + if let TyKind::TraitObject(ref bounds, TraitObjectSyntax::None) = ty.kind { + // We cannot disambiguate multi-segment paths right now as that requires type + // checking. + let const_expr_without_braces = bounds.iter().all(|bound| match bound { + GenericBound::Trait( + PolyTraitRef { + bound_generic_params, + trait_ref: TraitRef { path, .. }, + .. + }, + TraitBoundModifier::None, + ) if bound_generic_params.is_empty() + && path.segments.len() == 1 + && path.segments[0].args.is_none() => + { + let part_res = self.resolver.get_partial_res(path.segments[0].id); + match part_res.map(|r| r.base_res()) { + Some(res) => { + !res.matches_ns(Namespace::TypeNS) + && res.matches_ns(Namespace::ValueNS) + } + None => true, + } + } + _ => false, + }); + if const_expr_without_braces { + self.sess.struct_span_err(ty.span, "likely `const` expression parsed as trait bounds") + .span_label(ty.span, "parsed as trait bounds but traits weren't found") + .multipart_suggestion( + "if you meant to write a `const` expression, surround the expression with braces", + vec![ + (ty.span.shrink_to_lo(), "{ ".to_string()), + (ty.span.shrink_to_hi(), " }".to_string()), + ], + Applicability::MachineApplicable, + ) + .emit(); + + let parent_def_id = self.current_hir_id_owner.last().unwrap().0; + let node_id = self.resolver.next_node_id(); + // Add a definition for the in-band const def. + self.resolver.definitions().create_def_with_parent( + parent_def_id, + node_id, + DefPathData::AnonConst, + ExpnId::root(), + ty.span, + ); + + let path_expr = Expr { + id: ty.id, + kind: ExprKind::Err, + span: ty.span, + attrs: AttrVec::new(), + }; + let value = self.with_new_scopes(|this| hir::AnonConst { + hir_id: this.lower_node_id(node_id), + body: this.lower_const_body(path_expr.span, Some(&path_expr)), + }); + return GenericArg::Const(ConstArg { value, span: ty.span }); + } + } GenericArg::Type(self.lower_ty_direct(&ty, itctx)) } ast::GenericArg::Const(ct) => GenericArg::Const(ConstArg { diff --git a/src/librustc_middle/ty/sty.rs b/src/librustc_middle/ty/sty.rs index 2ad673b2c1943..242bd274eabd6 100644 --- a/src/librustc_middle/ty/sty.rs +++ b/src/librustc_middle/ty/sty.rs @@ -2268,6 +2268,7 @@ impl<'tcx> Const<'tcx> { let name = tcx.hir().name(hir_id); ty::ConstKind::Param(ty::ParamConst::new(index, name)) } + ExprKind::Err => ty::ConstKind::Error, _ => ty::ConstKind::Unevaluated( def_id.to_def_id(), InternalSubsts::identity_for_item(tcx, def_id.to_def_id()), diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index e541920e89ed4..3a9104acce82e 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -558,25 +558,22 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { let prev = replace(&mut self.diagnostic_metadata.currently_processing_generics, true); match arg { GenericArg::Type(ref ty) => { - // We parse const arguments as path types as we cannot distinguish them during - // parsing. We try to resolve that ambiguity by attempting resolution the type - // namespace first, and if that fails we try again in the value namespace. If - // resolution in the value namespace succeeds, we have an generic const argument on - // our hands. - if let TyKind::Path(ref qself, ref path) = ty.kind { - // We cannot disambiguate multi-segment paths right now as that requires type - // checking. - if path.segments.len() == 1 && path.segments[0].args.is_none() { - let mut check_ns = |ns| { - self.resolve_ident_in_lexical_scope( - path.segments[0].ident, - ns, - None, - path.span, - ) - .is_some() - }; - if !check_ns(TypeNS) && check_ns(ValueNS) { + let mut check_ns = |path: &Path, ns| { + self.resolve_ident_in_lexical_scope(path.segments[0].ident, ns, None, path.span) + .is_some() + && path.segments.len() == 1 + && path.segments[0].args.is_none() + }; + match ty.kind { + // We parse const arguments as path types as we cannot distinguish them during + // parsing. We try to resolve that ambiguity by attempting resolution the type + // namespace first, and if that fails we try again in the value namespace. If + // resolution in the value namespace succeeds, we have an generic const argument + // on our hands. + TyKind::Path(ref qself, ref path) => { + // We cannot disambiguate multi-segment paths right now as that requires type + // checking. + if !check_ns(path, TypeNS) && check_ns(path, ValueNS) { // This must be equivalent to `visit_anon_const`, but we cannot call it // directly due to visitor lifetimes so we have to copy-paste some code. self.with_constant_rib(|this| { @@ -597,6 +594,38 @@ impl<'a, 'ast> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast> { return; } } + + // Possible `a + b` expression that should be surrounded in braces but was + // parsed as trait bounds in a trait object. Suggest surrounding with braces. + TyKind::TraitObject(ref bounds, TraitObjectSyntax::None) => { + // We cannot disambiguate multi-segment paths right now as that requires + // type checking. + let const_expr_without_braces = bounds.iter().all(|bound| match bound { + GenericBound::Trait( + PolyTraitRef { + bound_generic_params, + trait_ref: TraitRef { path, .. }, + .. + }, + TraitBoundModifier::None, + ) if bound_generic_params.is_empty() => { + !check_ns(path, TypeNS) && check_ns(path, ValueNS) + } + _ => false, + }); + if const_expr_without_braces { + // This will be handled and emit an appropriate error in + // `rustc_ast_lowering::LoweringContext::lower_generic_arg`. We do not + // `visit_ty` in this case to avoid extra unnecessary output. + self.r.session.delay_span_bug( + ty.span, + "`const` expression parsed as trait bounds", + ); + self.diagnostic_metadata.currently_processing_generics = prev; + return; + } + } + _ => {} } self.visit_ty(ty); diff --git a/src/test/ui/const-generics/const-expression-missing-braces.rs b/src/test/ui/const-generics/const-expression-missing-braces.rs index d9a7054f6c4a4..60cf9472d1ab0 100644 --- a/src/test/ui/const-generics/const-expression-missing-braces.rs +++ b/src/test/ui/const-generics/const-expression-missing-braces.rs @@ -11,11 +11,7 @@ fn a() { fn b() { let bar = 3; foo::(); - //~^ ERROR expected trait, found local variable `bar` - //~| ERROR expected trait, found local variable `bar` - //~| ERROR wrong number of const arguments: expected 1, found 0 - //~| ERROR wrong number of type arguments: expected 0, found 1 - //~| WARNING trait objects without an explicit `dyn` are deprecated + //~^ ERROR likely `const` expression parsed as trait bounds } fn c() { let bar = 3; diff --git a/src/test/ui/const-generics/const-expression-missing-braces.stderr b/src/test/ui/const-generics/const-expression-missing-braces.stderr index 3e03053f21ed8..c1dcc865b4326 100644 --- a/src/test/ui/const-generics/const-expression-missing-braces.stderr +++ b/src/test/ui/const-generics/const-expression-missing-braces.stderr @@ -5,44 +5,21 @@ LL | foo::(); | ^ expected one of 8 possible tokens error: expected one of `,` or `>`, found `+` - --> $DIR/const-expression-missing-braces.rs:22:13 + --> $DIR/const-expression-missing-braces.rs:18:13 | LL | foo::<3 + 3>(); | ^ expected one of `,` or `>` -error[E0404]: expected trait, found local variable `bar` +error: likely `const` expression parsed as trait bounds --> $DIR/const-expression-missing-braces.rs:13:11 | LL | foo::(); - | ^^^ not a trait - -error[E0404]: expected trait, found local variable `bar` - --> $DIR/const-expression-missing-braces.rs:13:17 + | ^^^^^^^^^ parsed as trait bounds but traits weren't found | -LL | foo::(); - | ^^^ not a trait - -warning: trait objects without an explicit `dyn` are deprecated - --> $DIR/const-expression-missing-braces.rs:13:11 +help: if you meant to write a `const` expression, surround the expression with braces | -LL | foo::(); - | ^^^^^^^^^ help: use `dyn`: `dyn bar + bar` - | - = note: `#[warn(bare_trait_objects)]` on by default - -error[E0107]: wrong number of const arguments: expected 1, found 0 - --> $DIR/const-expression-missing-braces.rs:13:5 - | -LL | foo::(); - | ^^^^^^^^^^^^^^^^ expected 1 const argument - -error[E0107]: wrong number of type arguments: expected 0, found 1 - --> $DIR/const-expression-missing-braces.rs:13:11 - | -LL | foo::(); - | ^^^^^^^^^ unexpected type argument +LL | foo::<{ bar + bar }>(); + | ^ ^ -error: aborting due to 6 previous errors; 1 warning emitted +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0107, E0404. -For more information about an error, try `rustc --explain E0107`. From 2cc9b30f55537c221d85c632c61d65f4a2837919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 16 May 2020 09:42:53 -0700 Subject: [PATCH 3/3] Move some `ast_lowering` diagnostics code to `diagnostics.rs` --- src/librustc_ast_lowering/diagnostics.rs | 73 ++++++++++++++++++++++++ src/librustc_ast_lowering/lib.rs | 72 +++-------------------- 2 files changed, 80 insertions(+), 65 deletions(-) create mode 100644 src/librustc_ast_lowering/diagnostics.rs diff --git a/src/librustc_ast_lowering/diagnostics.rs b/src/librustc_ast_lowering/diagnostics.rs new file mode 100644 index 0000000000000..69702497d962a --- /dev/null +++ b/src/librustc_ast_lowering/diagnostics.rs @@ -0,0 +1,73 @@ +use crate::LoweringContext; +use rustc_ast::ast::{ + AttrVec, Expr, ExprKind, GenericBound, PolyTraitRef, TraitBoundModifier, TraitObjectSyntax, + TraitRef, Ty, TyKind, +}; +use rustc_ast::ptr::P; +use rustc_errors::Applicability; +use rustc_hir::def::Namespace; +use rustc_hir::definitions::DefPathData; +use rustc_hir::{AnonConst, ConstArg, GenericArg}; +use rustc_span::hygiene::ExpnId; + +impl<'a, 'hir> LoweringContext<'a, 'hir> { + /// Possible `a + b` expression that should be surrounded in braces but was parsed + /// as trait bounds in a trait object. Suggest surrounding with braces. + crate fn detect_const_expr_as_trait_object(&mut self, ty: &P) -> Option> { + if let TyKind::TraitObject(ref bounds, TraitObjectSyntax::None) = ty.kind { + // We cannot disambiguate multi-segment paths right now as that requires type + // checking. + let const_expr_without_braces = bounds.iter().all(|bound| match bound { + GenericBound::Trait( + PolyTraitRef { bound_generic_params, trait_ref: TraitRef { path, .. }, .. }, + TraitBoundModifier::None, + ) if bound_generic_params.is_empty() + && path.segments.len() == 1 + && path.segments[0].args.is_none() => + { + let part_res = self.resolver.get_partial_res(path.segments[0].id); + match part_res.map(|r| r.base_res()) { + Some(res) => { + !res.matches_ns(Namespace::TypeNS) && res.matches_ns(Namespace::ValueNS) + } + None => true, + } + } + _ => false, + }); + if const_expr_without_braces { + self.sess.struct_span_err(ty.span, "likely `const` expression parsed as trait bounds") + .span_label(ty.span, "parsed as trait bounds but traits weren't found") + .multipart_suggestion( + "if you meant to write a `const` expression, surround the expression with braces", + vec![ + (ty.span.shrink_to_lo(), "{ ".to_string()), + (ty.span.shrink_to_hi(), " }".to_string()), + ], + Applicability::MachineApplicable, + ) + .emit(); + + let parent_def_id = self.current_hir_id_owner.last().unwrap().0; + let node_id = self.resolver.next_node_id(); + // Add a definition for the in-band const def. + self.resolver.definitions().create_def_with_parent( + parent_def_id, + node_id, + DefPathData::AnonConst, + ExpnId::root(), + ty.span, + ); + + let path_expr = + Expr { id: ty.id, kind: ExprKind::Err, span: ty.span, attrs: AttrVec::new() }; + let value = self.with_new_scopes(|this| AnonConst { + hir_id: this.lower_node_id(node_id), + body: this.lower_const_body(path_expr.span, Some(&path_expr)), + }); + return Some(GenericArg::Const(ConstArg { value, span: ty.span })); + } + } + None + } +} diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index b65aa817f830b..9ba926e86bc08 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -49,7 +49,7 @@ use rustc_ast_pretty::pprust; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::sync::Lrc; -use rustc_errors::{struct_span_err, Applicability}; +use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Namespace, PartialRes, PerNS, Res}; use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId, CRATE_DEF_INDEX}; @@ -78,6 +78,7 @@ macro_rules! arena_vec { }); } +mod diagnostics; mod expr; mod item; mod pat; @@ -1136,70 +1137,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } } - // Possible `a + b` expression that should be surrounded in braces but was parsed - // as trait bounds in a trait object. Suggest surrounding with braces. - if let TyKind::TraitObject(ref bounds, TraitObjectSyntax::None) = ty.kind { - // We cannot disambiguate multi-segment paths right now as that requires type - // checking. - let const_expr_without_braces = bounds.iter().all(|bound| match bound { - GenericBound::Trait( - PolyTraitRef { - bound_generic_params, - trait_ref: TraitRef { path, .. }, - .. - }, - TraitBoundModifier::None, - ) if bound_generic_params.is_empty() - && path.segments.len() == 1 - && path.segments[0].args.is_none() => - { - let part_res = self.resolver.get_partial_res(path.segments[0].id); - match part_res.map(|r| r.base_res()) { - Some(res) => { - !res.matches_ns(Namespace::TypeNS) - && res.matches_ns(Namespace::ValueNS) - } - None => true, - } - } - _ => false, - }); - if const_expr_without_braces { - self.sess.struct_span_err(ty.span, "likely `const` expression parsed as trait bounds") - .span_label(ty.span, "parsed as trait bounds but traits weren't found") - .multipart_suggestion( - "if you meant to write a `const` expression, surround the expression with braces", - vec![ - (ty.span.shrink_to_lo(), "{ ".to_string()), - (ty.span.shrink_to_hi(), " }".to_string()), - ], - Applicability::MachineApplicable, - ) - .emit(); - - let parent_def_id = self.current_hir_id_owner.last().unwrap().0; - let node_id = self.resolver.next_node_id(); - // Add a definition for the in-band const def. - self.resolver.definitions().create_def_with_parent( - parent_def_id, - node_id, - DefPathData::AnonConst, - ExpnId::root(), - ty.span, - ); - - let path_expr = Expr { - id: ty.id, - kind: ExprKind::Err, - span: ty.span, - attrs: AttrVec::new(), - }; - let value = self.with_new_scopes(|this| hir::AnonConst { - hir_id: this.lower_node_id(node_id), - body: this.lower_const_body(path_expr.span, Some(&path_expr)), - }); - return GenericArg::Const(ConstArg { value, span: ty.span }); - } + if let Some(arg) = self.detect_const_expr_as_trait_object(ty) { + // Possible `a + b` expression that should be surrounded in braces but was + // parsed as trait bounds in a trait object. Suggest surrounding with braces + // and recover by returning err expression const argument. + return arg; } GenericArg::Type(self.lower_ty_direct(&ty, itctx)) }