Fixing span manipulation and indentation of the suggestion introduced…

… by #126187 According to: #128084 (comment) https://github.com/rust-lang/rust/pull/126187/files#r1634897691 I try to add a span field to `Block` which doesn't include brace '{' and '}', because I need to add a suggestion at the end of function's body, and this can help me find the right place. But this will make `Block` larger. I don't want to break the original span field in `Block`, I'm worried that it will cause a lot of code failures and I think it may be more intuitive for span to include braces.
rust-lang · Aug 22, 2024 · 63c6434 · 63c6434
1 parent 4d7c095
commit 63c6434
Show file tree

Hide file tree

Showing 19 changed files with 114 additions and 92 deletions.
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs
@@ -544,6 +544,9 @@ pub struct Block {
     /// Distinguishes between `unsafe { ... }` and `{ ... }`.
     pub rules: BlockCheckMode,
     pub span: Span,
+    // Only for function or method written by developers that do have a block,
+    // but not including the blocks automatically inserted by the compiler.
+    pub no_brace_span: Option<Span>,
     pub tokens: Option<LazyAttrTokenStream>,
     /// The following *isn't* a parse error, but will cause multiple errors in following stages.
     /// ```compile_fail
@@ -3501,7 +3504,7 @@ mod size_asserts {
     static_assert_size!(AssocItem, 88);
     static_assert_size!(AssocItemKind, 16);
     static_assert_size!(Attribute, 32);
-    static_assert_size!(Block, 32);
+    static_assert_size!(Block, 48);
     static_assert_size!(Expr, 72);
     static_assert_size!(ExprKind, 40);
     static_assert_size!(Fn, 160);

diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs
@@ -1076,7 +1076,8 @@ fn walk_mt<T: MutVisitor>(vis: &mut T, MutTy { ty, mutbl: _ }: &mut MutTy) {
 }
 
 pub fn walk_block<T: MutVisitor>(vis: &mut T, block: &mut P<Block>) {
-    let Block { id, stmts, rules: _, span, tokens, could_be_bare_literal: _ } = block.deref_mut();
+    let Block { id, stmts, rules: _, span, tokens, could_be_bare_literal: _, no_brace_span: _ } =
+        block.deref_mut();
     vis.visit_id(id);
     stmts.flat_map_in_place(|stmt| vis.flat_map_stmt(stmt));
     visit_lazy_tts(vis, tokens);

diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs
@@ -943,7 +943,15 @@ pub fn walk_field_def<'a, V: Visitor<'a>>(visitor: &mut V, field: &'a FieldDef)
 }
 
 pub fn walk_block<'a, V: Visitor<'a>>(visitor: &mut V, block: &'a Block) -> V::Result {
-    let Block { stmts, id: _, rules: _, span: _, tokens: _, could_be_bare_literal: _ } = block;
+    let Block {
+        stmts,
+        id: _,
+        rules: _,
+        span: _,
+        tokens: _,
+        could_be_bare_literal: _,
+        no_brace_span: _,
+    } = block;
     walk_list!(visitor, visit_stmt, stmts);
     V::Result::output()
 }

diff --git a/compiler/rustc_ast_lowering/src/block.rs b/compiler/rustc_ast_lowering/src/block.rs
@@ -1,5 +1,6 @@
 use rustc_ast::{Block, BlockCheckMode, Local, LocalKind, Stmt, StmtKind};
 use rustc_hir as hir;
+use rustc_span::Span;
 use smallvec::SmallVec;
 
 use crate::{ImplTraitContext, ImplTraitPosition, LoweringContext};
@@ -21,7 +22,17 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         let (stmts, expr) = self.lower_stmts(&b.stmts);
         let rules = self.lower_block_check_mode(&b.rules);
         let hir_id = self.lower_node_id(b.id);
-        hir::Block { hir_id, stmts, expr, rules, span: self.lower_span(b.span), targeted_by_break }
+        let lower = |span: Span| -> Option<Span> { Some(self.lower_span(span)) };
+        let no_brace_span = b.no_brace_span.and_then(lower);
+        hir::Block {
+            hir_id,
+            stmts,
+            expr,
+            rules,
+            span: self.lower_span(b.span),
+            no_brace_span,
+            targeted_by_break,
+        }
     }
 
     fn lower_stmts(

diff --git a/compiler/rustc_ast_lowering/src/delegation.rs b/compiler/rustc_ast_lowering/src/delegation.rs
@@ -353,6 +353,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
             hir_id: self.next_id(),
             rules: hir::BlockCheckMode::DefaultBlock,
             span,
+            no_brace_span: None,
             targeted_by_break: false,
         });
 

diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs
@@ -592,6 +592,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 hir_id: self.next_id(),
                 rules: hir::BlockCheckMode::DefaultBlock,
                 span,
+                no_brace_span: None,
                 targeted_by_break: false,
             });
             self.arena.alloc(hir::Expr {
@@ -2116,6 +2117,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     hir_id,
                     rules: hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated),
                     span: self.lower_span(span),
+                    no_brace_span: None,
                     targeted_by_break: false,
                 }),
                 None,

diff --git a/compiler/rustc_ast_lowering/src/format.rs b/compiler/rustc_ast_lowering/src/format.rs
@@ -612,6 +612,7 @@ fn expand_format_args<'hir>(
             hir_id,
             rules: hir::BlockCheckMode::UnsafeBlock(hir::UnsafeSource::CompilerGenerated),
             span: macsp,
+            no_brace_span: None,
             targeted_by_break: false,
         }));
         let args = ctx.arena.alloc_from_iter([lit_pieces, args, format_options, unsafe_arg]);

diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs
@@ -2571,6 +2571,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             hir_id: self.next_id(),
             rules: hir::BlockCheckMode::DefaultBlock,
             span: self.lower_span(span),
+            no_brace_span: None,
             targeted_by_break: false,
         };
         self.arena.alloc(blk)

diff --git a/compiler/rustc_builtin_macros/src/deriving/mod.rs b/compiler/rustc_builtin_macros/src/deriving/mod.rs
@@ -112,6 +112,7 @@ fn call_unreachable(cx: &ExtCtxt<'_>, span: Span) -> P<ast::Expr> {
         id: ast::DUMMY_NODE_ID,
         rules: ast::BlockCheckMode::Unsafe(ast::CompilerGenerated),
         span,
+        no_brace_span: None,
         tokens: None,
         could_be_bare_literal: false,
     }))

diff --git a/compiler/rustc_expand/src/build.rs b/compiler/rustc_expand/src/build.rs
@@ -247,6 +247,7 @@ impl<'a> ExtCtxt<'a> {
             id: ast::DUMMY_NODE_ID,
             rules: BlockCheckMode::Default,
             span,
+            no_brace_span: None,
             tokens: None,
             could_be_bare_literal: false,
         })

diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs
@@ -1095,6 +1095,10 @@ pub struct Block<'hir> {
     pub rules: BlockCheckMode,
     /// The span includes the curly braces `{` and `}` around the block.
     pub span: Span,
+    // The span doesn't include the curly braces `{` and `}` around the block.
+    // Only for function or method written by developers that do have a block,
+    // but not including the blocks automatically inserted by the compiler.
+    pub no_brace_span: Option<Span>,
     /// If true, then there may exist `break 'a` values that aim to
     /// break out of this block early.
     /// Used by `'label: {}` blocks and by `try {}` blocks.
@@ -4040,7 +4044,7 @@ mod size_asserts {
 
     use super::*;
     // tidy-alphabetical-start
-    static_assert_size!(Block<'_>, 48);
+    static_assert_size!(Block<'_>, 56);
     static_assert_size!(Body<'_>, 24);
     static_assert_size!(Expr<'_>, 64);
     static_assert_size!(ExprKind<'_>, 48);

diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs
@@ -928,6 +928,7 @@ impl<'a> Parser<'a> {
                         thin_vec![self.mk_stmt_err(expr.span, guar)],
                         s,
                         lo.to(self.prev_token.span),
+                        None,
                     );
                     tail.could_be_bare_literal = true;
                     if maybe_struct_name.is_ident() && can_be_struct_literal {

diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
@@ -1731,7 +1731,7 @@ impl<'a> Parser<'a> {
 
                 // Replace `'label: non_block_expr` with `'label: {non_block_expr}` in order to suppress future errors about `break 'label`.
                 let stmt = self.mk_stmt(span, StmtKind::Expr(expr));
-                let blk = self.mk_block(thin_vec![stmt], BlockCheckMode::Default, span);
+                let blk = self.mk_block(thin_vec![stmt], BlockCheckMode::Default, span, None);
                 self.mk_expr(span, ExprKind::Block(blk, label))
             });
 
@@ -2847,7 +2847,8 @@ impl<'a> Parser<'a> {
                 .dcx()
                 .emit_err(errors::MissingExpressionInForLoop { span: expr.span.shrink_to_lo() });
             let err_expr = self.mk_expr(expr.span, ExprKind::Err(guar));
-            let block = self.mk_block(thin_vec![], BlockCheckMode::Default, self.prev_token.span);
+            let block =
+                self.mk_block(thin_vec![], BlockCheckMode::Default, self.prev_token.span, None);
             return Ok(self.mk_expr(
                 lo.to(self.prev_token.span),
                 ExprKind::ForLoop { pat, iter: err_expr, body: block, label: opt_label, kind },

diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs
@@ -578,6 +578,8 @@ impl<'a> Parser<'a> {
     ) -> PResult<'a, P<Block>> {
         let mut stmts = ThinVec::new();
         let mut snapshot = None;
+        let start_without_brace_sp = self.token.span;
+        let mut end_without_brace_sp = self.token.span;
         while !self.eat(&token::CloseDelim(Delimiter::Brace)) {
             if self.token == token::Eof {
                 break;
@@ -641,8 +643,14 @@ impl<'a> Parser<'a> {
                 // Found only `;` or `}`.
                 continue;
             };
+            end_without_brace_sp = self.prev_token.span;
         }
-        Ok(self.mk_block(stmts, s, lo.to(self.prev_token.span)))
+        Ok(self.mk_block(
+            stmts,
+            s,
+            lo.to(self.prev_token.span),
+            Some(start_without_brace_sp.to(end_without_brace_sp)),
+        ))
     }
 
     /// Parses a statement, including the trailing semicolon.
@@ -843,12 +851,14 @@ impl<'a> Parser<'a> {
         stmts: ThinVec<Stmt>,
         rules: BlockCheckMode,
         span: Span,
+        no_brace_span: Option<Span>,
     ) -> P<Block> {
         P(Block {
             stmts,
             id: DUMMY_NODE_ID,
             rules,
             span,
+            no_brace_span,
             tokens: None,
             could_be_bare_literal: false,
         })
@@ -863,6 +873,6 @@ impl<'a> Parser<'a> {
     }
 
     pub(super) fn mk_block_err(&self, span: Span, guar: ErrorGuaranteed) -> P<Block> {
-        self.mk_block(thin_vec![self.mk_stmt_err(span, guar)], BlockCheckMode::Default, span)
+        self.mk_block(thin_vec![self.mk_stmt_err(span, guar)], BlockCheckMode::Default, span, None)
     }
 }
diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/suggestions.rs
@@ -4666,12 +4666,9 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
             let body = self.tcx.hir().body(body_id);
             if let hir::ExprKind::Block(b, _) = body.value.kind
                 && b.expr.is_none()
+                && let Some(span) = b.no_brace_span
             {
-                sugg_spans.push((
-                    // The span will point to the closing curly brace `}` of the block.
-                    b.span.shrink_to_hi().with_lo(b.span.hi() - BytePos(1)),
-                    "\n    Ok(())\n}".to_string(),
-                ));
+                sugg_spans.push((span.shrink_to_hi(), "\n    Ok(())".to_string()));
             }
             err.multipart_suggestion_verbose(
                 format!("consider adding return type"),

diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.fixed b/tests/ui/return/return-from-residual-sugg-issue-125997.fixed
@@ -8,16 +8,14 @@ use std::io::prelude::*;
 
 fn test1() -> Result<(), Box<dyn std::error::Error>> {
     let mut _file = File::create("foo.txt")?;
-    //~^ ERROR the `?` operator can only be used in a function
-
     Ok(())
+    //~^ ERROR the `?` operator can only be used in a function
 }
 
 fn test2() -> Result<(), Box<dyn std::error::Error>> {
     let mut _file = File::create("foo.txt")?;
     //~^ ERROR the `?` operator can only be used in a function
     println!();
-
     Ok(())
 }
 
@@ -27,9 +25,8 @@ macro_rules! mac {
             let mut _file = File::create("foo.txt")?;
             //~^ ERROR the `?` operator can only be used in a function
             println!();
-
     Ok(())
-}
+        }
     };
 }
 
@@ -38,24 +35,21 @@ struct A;
 impl A {
     fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
         let mut _file = File::create("foo.txt")?;
-        //~^ ERROR the `?` operator can only be used in a method
-
     Ok(())
-}
+        //~^ ERROR the `?` operator can only be used in a method
+    }
 
     fn test5(&self) -> Result<(), Box<dyn std::error::Error>> {
         let mut _file = File::create("foo.txt")?;
         //~^ ERROR the `?` operator can only be used in a method
         println!();
-
     Ok(())
-}
+    }
 }
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mut _file = File::create("foo.txt")?;
     //~^ ERROR the `?` operator can only be used in a function
     mac!();
-
     Ok(())
 }
diff --git a/tests/ui/return/return-from-residual-sugg-issue-125997.stderr b/tests/ui/return/return-from-residual-sugg-issue-125997.stderr
@@ -10,11 +10,8 @@ LL |     let mut _file = File::create("foo.txt")?;
 help: consider adding return type
    |
 LL ~ fn test1() -> Result<(), Box<dyn std::error::Error>> {
-LL |     let mut _file = File::create("foo.txt")?;
-LL |
-LL + 
+LL ~     let mut _file = File::create("foo.txt")?;
 LL +     Ok(())
-LL + }
    |
 
 error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -31,10 +28,8 @@ help: consider adding return type
 LL ~ fn test2() -> Result<(), Box<dyn std::error::Error>> {
 LL |     let mut _file = File::create("foo.txt")?;
 LL |
-LL |     println!();
-LL + 
+LL ~     println!();
 LL +     Ok(())
-LL + }
    |
 
 error[E0277]: the `?` operator can only be used in a method that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -49,11 +44,8 @@ LL |         let mut _file = File::create("foo.txt")?;
 help: consider adding return type
    |
 LL ~     fn test4(&self) -> Result<(), Box<dyn std::error::Error>> {
-LL |         let mut _file = File::create("foo.txt")?;
-LL |
-LL ~     
+LL ~         let mut _file = File::create("foo.txt")?;
 LL +     Ok(())
-LL + }
    |
 
 error[E0277]: the `?` operator can only be used in a method that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -70,10 +62,8 @@ help: consider adding return type
 LL ~     fn test5(&self) -> Result<(), Box<dyn std::error::Error>> {
 LL |         let mut _file = File::create("foo.txt")?;
 LL |
-LL |         println!();
-LL ~     
+LL ~         println!();
 LL +     Ok(())
-LL + }
    |
 
 error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -90,10 +80,8 @@ help: consider adding return type
 LL ~ fn main() -> Result<(), Box<dyn std::error::Error>> {
 LL |     let mut _file = File::create("foo.txt")?;
 LL |
-LL |     mac!();
-LL + 
+LL ~     mac!();
 LL +     Ok(())
-LL + }
    |
 
 error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `FromResidual`)
@@ -114,10 +102,8 @@ help: consider adding return type
 LL ~         fn test3() -> Result<(), Box<dyn std::error::Error>> {
 LL |             let mut _file = File::create("foo.txt")?;
 LL |
-LL |             println!();
-LL ~         
+LL ~             println!();
 LL +     Ok(())
-LL + }
    |
 
 error: aborting due to 6 previous errors