https://gcc.gnu.org/g:8c54e7eeec04a924f6ea63b51e52e24b1fdae95f
commit r16-4772-g8c54e7eeec04a924f6ea63b51e52e24b1fdae95f Author: Owen Avery <[email protected]> Date: Sat Aug 9 18:33:58 2025 -0400 gccrs: Add checks to ExpandVisitor This should help detect issues like https://github.com/Rust-GCC/gccrs/issues/3444. gcc/rust/ChangeLog: * ast/rust-ast.h (Stmt::get_node_id): Make virtual. (Type::get_node_id): Likewise. (AssociatedItem::get_node_id): New virtual member function. * ast/rust-expr.h (TypeCastExpr::get_casted_expr_ptr): New member function. (TypeCastExpr::get_type_to_cast_to_ptr): Likewise. (ClosureExprInner::get_definition_expr_ptr): Likewise. * ast/rust-item.h (TypeAlias::get_node_id): New member function to override AssociatedItem::get_node_id. (ConstantItem::get_node_id): Likewise. * expand/rust-expand-visitor.cc (ExpandVisitor::maybe_expand_expr): Adjust macro_invoc_expect_id. (ExpandVisitor::maybe_expand_type): Likewise and add an overload for std::unique_ptr<TypeNoBounds>. (ExpandVisitor::visit): Check macro_invoc_expect_id and generally improve visitors so that the testsuite will still pass. * expand/rust-expand-visitor.h (ExpandVisitor::ExpandVisitor): Initialize member variable macro_invoc_expect_id. (ExpandVisitor::maybe_expand_type): Add an overload for std::unique_ptr<TypeNoBounds>. (ExpandVisitor::expand_macro_children): Adjust macro_invoc_expect_id. (ExpandVisitor::visit): Add an overload for TypeCastExpr. (ExpandVisitor::macro_invoc_expect_id): New member variable. gcc/testsuite/ChangeLog: * rust/compile/macros/mbe/macro49.rs: Add missing lang items. Signed-off-by: Owen Avery <[email protected]> Diff: --- gcc/rust/ast/rust-ast.h | 9 ++++- gcc/rust/ast/rust-expr.h | 18 +++++++++ gcc/rust/ast/rust-item.h | 6 +++ gcc/rust/expand/rust-expand-visitor.cc | 47 +++++++++++++++++++++++- gcc/rust/expand/rust-expand-visitor.h | 10 ++++- gcc/testsuite/rust/compile/macros/mbe/macro49.rs | 11 ++++++ 6 files changed, 96 insertions(+), 5 deletions(-) diff --git a/gcc/rust/ast/rust-ast.h b/gcc/rust/ast/rust-ast.h index 2d2c5d0b4d76..0feaf5141050 100644 --- a/gcc/rust/ast/rust-ast.h +++ b/gcc/rust/ast/rust-ast.h @@ -1137,7 +1137,9 @@ public: virtual void mark_for_strip () = 0; virtual bool is_marked_for_strip () const = 0; - NodeId get_node_id () const { return node_id; } + + // TODO: put this in a virtual base class? + virtual NodeId get_node_id () const { return node_id; } virtual Kind get_stmt_kind () = 0; @@ -1536,7 +1538,8 @@ public: virtual location_t get_locus () const = 0; - NodeId get_node_id () const { return node_id; } + // TODO: put this in a virtual base class? + virtual NodeId get_node_id () const { return node_id; } virtual Type *reconstruct_impl () const = 0; protected: @@ -1799,6 +1802,8 @@ public: virtual bool is_marked_for_strip () const = 0; virtual location_t get_locus () const = 0; + + virtual NodeId get_node_id () const = 0; }; // Item used in trait declarations - abstract base class diff --git a/gcc/rust/ast/rust-expr.h b/gcc/rust/ast/rust-expr.h index 03cad86de3d0..8e5abe15219c 100644 --- a/gcc/rust/ast/rust-expr.h +++ b/gcc/rust/ast/rust-expr.h @@ -865,6 +865,12 @@ public: return *main_or_left_expr; } + std::unique_ptr<Expr> &get_casted_expr_ptr () + { + rust_assert (main_or_left_expr != nullptr); + return main_or_left_expr; + } + // TODO: is this better? Or is a "vis_block" better? TypeNoBounds &get_type_to_cast_to () { @@ -872,6 +878,12 @@ public: return *type_to_convert_to; } + std::unique_ptr<TypeNoBounds> &get_type_to_cast_to_ptr () + { + rust_assert (type_to_convert_to != nullptr); + return type_to_convert_to; + } + Expr::Kind get_expr_kind () const override { return Expr::Kind::TypeCast; } protected: @@ -2597,6 +2609,12 @@ public: return *closure_inner; } + std::unique_ptr<Expr> &get_definition_expr_ptr () + { + rust_assert (closure_inner != nullptr); + return closure_inner; + } + protected: /* Use covariance to implement clone function as returning this object rather * than base */ diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h index d11eed7687b5..375f66ddcc1e 100644 --- a/gcc/rust/ast/rust-item.h +++ b/gcc/rust/ast/rust-item.h @@ -1567,6 +1567,9 @@ public: location_t get_locus () const override final { return locus; } + // needed to override AssociatedItem::get_node_id + NodeId get_node_id () const override final { return VisItem::get_node_id (); } + void accept_vis (ASTVisitor &vis) override; // Invalid if existing type is null, so base stripping on that. @@ -2515,6 +2518,9 @@ public: location_t get_locus () const override final { return locus; } + // needed to override AssociatedItem::get_node_id + NodeId get_node_id () const override final { return VisItem::get_node_id (); } + void accept_vis (ASTVisitor &vis) override; // Invalid if type or expression are null, so base stripping on that. diff --git a/gcc/rust/expand/rust-expand-visitor.cc b/gcc/rust/expand/rust-expand-visitor.cc index 8f6e7faa0c45..a53f0640109d 100644 --- a/gcc/rust/expand/rust-expand-visitor.cc +++ b/gcc/rust/expand/rust-expand-visitor.cc @@ -329,10 +329,15 @@ ExpandVisitor::expand_inner_stmts (AST::BlockExpr &expr) void ExpandVisitor::maybe_expand_expr (std::unique_ptr<AST::Expr> &expr) { + NodeId old_expect = expr->get_node_id (); + std::swap (macro_invoc_expect_id, old_expect); + expander.push_context (MacroExpander::ContextType::EXPR); expr->accept_vis (*this); expander.pop_context (); + std::swap (macro_invoc_expect_id, old_expect); + auto final_fragment = expander.take_expanded_fragment (); if (final_fragment.should_expand () && final_fragment.is_expression_fragment ()) @@ -342,14 +347,37 @@ ExpandVisitor::maybe_expand_expr (std::unique_ptr<AST::Expr> &expr) void ExpandVisitor::maybe_expand_type (std::unique_ptr<AST::Type> &type) { - expander.push_context (MacroExpander::ContextType::TYPE); + NodeId old_expect = type->get_node_id (); + std::swap (macro_invoc_expect_id, old_expect); + expander.push_context (MacroExpander::ContextType::TYPE); type->accept_vis (*this); + expander.pop_context (); + + std::swap (macro_invoc_expect_id, old_expect); + auto final_fragment = expander.take_expanded_fragment (); if (final_fragment.should_expand () && final_fragment.is_type_fragment ()) type = final_fragment.take_type_fragment (); +} + +// HACK: maybe we shouldn't have TypeNoBounds as a base class +void +ExpandVisitor::maybe_expand_type (std::unique_ptr<AST::TypeNoBounds> &type) +{ + NodeId old_expect = type->get_node_id (); + std::swap (macro_invoc_expect_id, old_expect); + expander.push_context (MacroExpander::ContextType::TYPE); + type->accept_vis (*this); expander.pop_context (); + + std::swap (macro_invoc_expect_id, old_expect); + + auto final_fragment = expander.take_expanded_fragment (); + if (final_fragment.should_expand () && final_fragment.is_type_fragment ()) + type = std::make_unique<AST::ParenthesisedType> ( + final_fragment.take_type_fragment (), BUILTINS_LOCATION); } // FIXME: Can this be refactored into a `scoped` method? Which takes a @@ -465,6 +493,14 @@ ExpandVisitor::visit (AST::ConstGenericParam &) void ExpandVisitor::visit (AST::MacroInvocation ¯o_invoc) { + if (macro_invoc_expect_id != macro_invoc.get_node_id ()) + { + rust_internal_error_at ( + macro_invoc.get_locus (), + "attempting to expand node with id %d into position with node id %d", + (int) macro_invoc.get_node_id (), (int) macro_invoc_expect_id); + } + // TODO: Can we do the AST fragment replacing here? Probably not, right? expander.expand_invoc (macro_invoc, macro_invoc.has_semicolon () ? AST::InvocKind::Semicoloned @@ -568,6 +604,13 @@ ExpandVisitor::visit (AST::LazyBooleanExpr &expr) maybe_expand_expr (expr.get_right_expr_ptr ()); } +void +ExpandVisitor::visit (AST::TypeCastExpr &expr) +{ + maybe_expand_expr (expr.get_casted_expr_ptr ()); + maybe_expand_type (expr.get_type_to_cast_to_ptr ()); +} + void ExpandVisitor::visit (AST::AssignmentExpr &expr) { @@ -615,7 +658,7 @@ ExpandVisitor::visit (AST::ClosureExprInner &expr) { expand_closure_params (expr.get_params ()); - visit (expr.get_definition_expr ()); + maybe_expand_expr (expr.get_definition_expr_ptr ()); } void diff --git a/gcc/rust/expand/rust-expand-visitor.h b/gcc/rust/expand/rust-expand-visitor.h index 845e10cfec78..01a0d1cb2975 100644 --- a/gcc/rust/expand/rust-expand-visitor.h +++ b/gcc/rust/expand/rust-expand-visitor.h @@ -38,7 +38,9 @@ bool is_builtin (AST::Attribute &attr); class ExpandVisitor : public AST::DefaultASTVisitor { public: - ExpandVisitor (MacroExpander &expander) : expander (expander) {} + ExpandVisitor (MacroExpander &expander) + : expander (expander), macro_invoc_expect_id (UNKNOWN_NODEID) + {} /* Expand all of the macro invocations currently contained in a crate */ void go (AST::Crate &crate); @@ -56,6 +58,7 @@ public: type : Core guidelines R33, this function reseat the pointer. */ void maybe_expand_type (std::unique_ptr<AST::Type> &type); + void maybe_expand_type (std::unique_ptr<AST::TypeNoBounds> &type); /** * Expand all macro invocations in lieu of types within a vector of struct @@ -128,7 +131,10 @@ public: auto &value = *it; // Perform expansion + NodeId old_expect = value->get_node_id (); + std::swap (macro_invoc_expect_id, old_expect); value->accept_vis (*this); + std::swap (macro_invoc_expect_id, old_expect); auto final_fragment = expander.take_expanded_fragment (); @@ -214,6 +220,7 @@ public: void visit (AST::ArithmeticOrLogicalExpr &expr) override; void visit (AST::ComparisonExpr &expr) override; void visit (AST::LazyBooleanExpr &expr) override; + void visit (AST::TypeCastExpr &expr) override; void visit (AST::AssignmentExpr &expr) override; void visit (AST::CompoundAssignmentExpr &expr) override; void visit (AST::GroupedExpr &expr) override; @@ -287,6 +294,7 @@ public: private: MacroExpander &expander; + NodeId macro_invoc_expect_id; }; } // namespace Rust diff --git a/gcc/testsuite/rust/compile/macros/mbe/macro49.rs b/gcc/testsuite/rust/compile/macros/mbe/macro49.rs index 0900f7cecdb6..9d63ff1e627d 100644 --- a/gcc/testsuite/rust/compile/macros/mbe/macro49.rs +++ b/gcc/testsuite/rust/compile/macros/mbe/macro49.rs @@ -1,3 +1,14 @@ +#[lang = "sized"] +pub trait Sized {} + +#[lang = "fn_once"] +trait FnOnce<Args> { + #[lang = "fn_once_output"] + type Output; + + extern "rust-call" fn call_once(self, args: Args) -> Self::Output; +} + macro_rules! closure { () => {{ 14 + 15
