From a6f31be825f6bd1807f033bf5fd7ccd0df6fd521 Mon Sep 17 00:00:00 2001 From: Pierre-Emmanuel Patry Date: Mon, 28 Oct 2024 18:08:52 +0100 Subject: [PATCH] Fixes some tests appearing with a moved variant A variant being moved lead to a null being created and a segfault later down the line. gcc/rust/ChangeLog: * backend/rust-compile-expr.cc (CompileExpr::visit): Call getter instead of size function. * checks/errors/privacy/rust-privacy-reporter.cc (PrivacyReporter::visit): Only check privacy if the type is present. * hir/rust-ast-lower-stmt.cc (ASTLoweringStmt::visit): Use an optional. * hir/tree/rust-hir-generic-param.h: Assert type before getting it. * hir/tree/rust-hir-item.h: Assert pointers before dereference, fix has_type condition. * hir/tree/rust-hir-path.h: Add more assertions. * hir/tree/rust-hir-stmt.cc: Change constructor with optionals. * hir/tree/rust-hir-stmt.h: Use optionals over smart pointers to emphasize these fields might be missing. * hir/tree/rust-hir.cc (LetStmt::as_string): Use getters. * typecheck/rust-hir-type-check-expr.cc: Clone structures to prevent parent's fields from being nulled by the move operation. * typecheck/rust-hir-type-check-item.cc (TypeCheckItem::visit): Use optionals. * typecheck/rust-tyty.cc: Likewise. * typecheck/rust-tyty.h: Likewise. Signed-off-by: Pierre-Emmanuel Patry --- gcc/rust/backend/rust-compile-expr.cc | 2 +- .../errors/privacy/rust-privacy-reporter.cc | 6 +- gcc/rust/hir/rust-ast-lower-stmt.cc | 20 +++--- gcc/rust/hir/tree/rust-hir-generic-param.h | 6 +- gcc/rust/hir/tree/rust-hir-item.h | 69 +++++++++++++++---- gcc/rust/hir/tree/rust-hir-path.h | 18 ++++- gcc/rust/hir/tree/rust-hir-stmt.cc | 26 ++++--- gcc/rust/hir/tree/rust-hir-stmt.h | 40 ++++++++--- gcc/rust/hir/tree/rust-hir.cc | 4 +- .../typecheck/rust-hir-type-check-expr.cc | 5 +- .../typecheck/rust-hir-type-check-item.cc | 7 +- gcc/rust/typecheck/rust-tyty.cc | 40 ++++++++--- gcc/rust/typecheck/rust-tyty.h | 12 +++- 13 files changed, 189 insertions(+), 66 deletions(-) diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc index 35a6f7044090..c4ae79dbde27 100644 --- a/gcc/rust/backend/rust-compile-expr.cc +++ b/gcc/rust/backend/rust-compile-expr.cc @@ -1200,7 +1200,7 @@ CompileExpr::visit (HIR::CallExpr &expr) // this assumes all fields are in order from type resolution and if a // base struct was specified those fields are filed via accessors std::vector arguments; - for (size_t i = 0; i < expr.get_arguments ().size (); i++) + for (size_t i = 0; i < expr.num_params (); i++) { auto &argument = expr.get_arguments ().at (i); auto rvalue = CompileExpr::Compile (*argument, ctx); diff --git a/gcc/rust/checks/errors/privacy/rust-privacy-reporter.cc b/gcc/rust/checks/errors/privacy/rust-privacy-reporter.cc index 39212bf97f51..0c3c9f64bca1 100644 --- a/gcc/rust/checks/errors/privacy/rust-privacy-reporter.cc +++ b/gcc/rust/checks/errors/privacy/rust-privacy-reporter.cc @@ -741,9 +741,11 @@ PrivacyReporter::visit (HIR::EmptyStmt &) void PrivacyReporter::visit (HIR::LetStmt &stmt) { - check_type_privacy (stmt.get_type ()); + if (stmt.has_type ()) + check_type_privacy (stmt.get_type ()); - stmt.get_init_expr ().accept_vis (*this); + if (stmt.has_init_expr ()) + stmt.get_init_expr ().accept_vis (*this); } void diff --git a/gcc/rust/hir/rust-ast-lower-stmt.cc b/gcc/rust/hir/rust-ast-lower-stmt.cc index 8f8a1a9a4d57..07283941328f 100644 --- a/gcc/rust/hir/rust-ast-lower-stmt.cc +++ b/gcc/rust/hir/rust-ast-lower-stmt.cc @@ -16,6 +16,7 @@ // along with GCC; see the file COPYING3. If not see // . +#include "optional.h" #include "rust-ast-lower-item.h" #include "rust-ast-lower-stmt.h" #include "rust-ast-lower-type.h" @@ -68,12 +69,16 @@ ASTLoweringStmt::visit (AST::LetStmt &stmt) { HIR::Pattern *variables = ASTLoweringPattern::translate (stmt.get_pattern (), true); - HIR::Type *type = stmt.has_type () - ? ASTLoweringType::translate (stmt.get_type ()) - : nullptr; - HIR::Expr *init_expression - = stmt.has_init_expr () ? ASTLoweringExpr::translate (stmt.get_init_expr ()) - : nullptr; + + auto type + = stmt.has_type () ? tl::optional> ( + std::unique_ptr (ASTLoweringType::translate (stmt.get_type ()))) + : tl::nullopt; + auto init_expression + = stmt.has_init_expr () + ? tl::optional> (std::unique_ptr ( + ASTLoweringExpr::translate (stmt.get_init_expr ()))) + : tl::nullopt; auto crate_num = mappings.get_current_crate (); Analysis::NodeMapping mapping (crate_num, stmt.get_node_id (), @@ -81,8 +86,7 @@ ASTLoweringStmt::visit (AST::LetStmt &stmt) UNKNOWN_LOCAL_DEFID); translated = new HIR::LetStmt (mapping, std::unique_ptr (variables), - std::unique_ptr (init_expression), - std::unique_ptr (type), + std::move (init_expression), std::move (type), stmt.get_outer_attrs (), stmt.get_locus ()); } diff --git a/gcc/rust/hir/tree/rust-hir-generic-param.h b/gcc/rust/hir/tree/rust-hir-generic-param.h index 73b93d48a49f..a1c59bf0d6bf 100644 --- a/gcc/rust/hir/tree/rust-hir-generic-param.h +++ b/gcc/rust/hir/tree/rust-hir-generic-param.h @@ -156,7 +156,11 @@ class ConstGenericParam : public GenericParam bool has_default_expression () { return default_expression != nullptr; } std::string get_name () { return name; } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } Expr &get_default_expression () { return *default_expression; } protected: diff --git a/gcc/rust/hir/tree/rust-hir-item.h b/gcc/rust/hir/tree/rust-hir-item.h index 37c5313869ce..567432ecb325 100644 --- a/gcc/rust/hir/tree/rust-hir-item.h +++ b/gcc/rust/hir/tree/rust-hir-item.h @@ -24,6 +24,7 @@ #include "rust-common.h" #include "rust-hir-visibility.h" #include "rust-hir-generic-param.h" +#include "rust-system.h" namespace Rust { namespace HIR { @@ -141,7 +142,11 @@ class TypeParam : public GenericParam Identifier get_type_representation () const { return type_representation; } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } Analysis::NodeMapping get_type_mappings () const; @@ -414,7 +419,11 @@ struct SelfParam ImplicitSelfKind get_self_kind () const { return self_kind; } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } Analysis::NodeMapping get_mappings () { return mappings; } @@ -482,7 +491,11 @@ struct FunctionParam Pattern &get_param_name () { return *param_name; } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } const Analysis::NodeMapping &get_mappings () const { return mappings; } }; @@ -1112,7 +1125,11 @@ class TypeAlias : public VisItem, public ImplItem WhereClause &get_where_clause () { return where_clause; } - Type &get_type_aliased () { return *existing_type; } + Type &get_type_aliased () + { + rust_assert (existing_type); + return *existing_type; + } Identifier get_new_type_name () const { return new_type_name; } @@ -1766,7 +1783,11 @@ class ConstantItem : public VisItem, public ImplItem void accept_vis (HIRImplVisitor &vis) override; void accept_vis (HIRVisItemVisitor &vis) override; - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } Expr &get_expr () { return *const_expr; } @@ -1844,9 +1865,17 @@ class StaticItem : public VisItem bool is_mut () const { return mut == Mutability::Mut; } - Expr &get_expr () { return *expr; } + Expr &get_expr () + { + rust_assert (expr); + return *expr; + } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } ItemKind get_item_kind () const override { return ItemKind::Static; } @@ -2028,9 +2057,17 @@ class TraitItemConst : public TraitItem bool has_expr () const { return expr != nullptr; } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } - Expr &get_expr () { return *expr; } + Expr &get_expr () + { + rust_assert (expr); + return *expr; + } const std::string trait_identifier () const override final { @@ -2274,9 +2311,13 @@ class ImplBlock : public VisItem, public WithInnerAttrs location_t get_locus () const override final { return locus; } - Type &get_type () { return *impl_type; }; + Type &get_type () + { + rust_assert (impl_type); + return *impl_type; + }; - bool has_type () { return impl_type == nullptr; } + bool has_type () { return impl_type != nullptr; } std::vector> &get_generic_params () { @@ -2436,7 +2477,11 @@ struct NamedFunctionParam Identifier get_param_name () const { return name; } - Type &get_type () { return *param_type; } + Type &get_type () + { + rust_assert (param_type); + return *param_type; + } Analysis::NodeMapping get_mappings () const { return mappings; } }; diff --git a/gcc/rust/hir/tree/rust-hir-path.h b/gcc/rust/hir/tree/rust-hir-path.h index 4839ca6442d3..df5fd0c4e469 100644 --- a/gcc/rust/hir/tree/rust-hir-path.h +++ b/gcc/rust/hir/tree/rust-hir-path.h @@ -99,8 +99,16 @@ class GenericArgsBinding Identifier &get_identifier () { return identifier; } const Identifier &get_identifier () const { return identifier; } - Type &get_type () { return *type; } - const Type &get_type () const { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } + const Type &get_type () const + { + rust_assert (type); + return *type; + } location_t get_locus () const { return locus; } }; @@ -633,7 +641,11 @@ class QualifiedPathType Analysis::NodeMapping get_mappings () const { return mappings; } - Type &get_type () { return *type; } + Type &get_type () + { + rust_assert (type); + return *type; + } TypePath &get_trait () { return *trait; } diff --git a/gcc/rust/hir/tree/rust-hir-stmt.cc b/gcc/rust/hir/tree/rust-hir-stmt.cc index 764ea118cebe..025f67e2c9b1 100644 --- a/gcc/rust/hir/tree/rust-hir-stmt.cc +++ b/gcc/rust/hir/tree/rust-hir-stmt.cc @@ -17,13 +17,16 @@ // . #include "rust-hir-stmt.h" +#include "optional.h" +#include "rust-system.h" namespace Rust { namespace HIR { LetStmt::LetStmt (Analysis::NodeMapping mappings, std::unique_ptr variables_pattern, - std::unique_ptr init_expr, std::unique_ptr type, + tl::optional> init_expr, + tl::optional> type, AST::AttrVec outer_attrs, location_t locus) : Stmt (std::move (mappings)), outer_attrs (std::move (outer_attrs)), variables_pattern (std::move (variables_pattern)), type (std::move (type)), @@ -38,10 +41,13 @@ LetStmt::LetStmt (LetStmt const &other) variables_pattern = other.variables_pattern->clone_pattern (); // guard to prevent null dereference (always required) - if (other.init_expr != nullptr) - init_expr = other.init_expr->clone_expr (); - if (other.type != nullptr) - type = other.type->clone_type (); + if (other.has_init_expr ()) + init_expr = other.get_init_expr ().clone_expr (); + + if (other.has_type ()) + type = other.get_type ().clone_type (); + else + type = tl::nullopt; } LetStmt & @@ -57,14 +63,14 @@ LetStmt::operator= (LetStmt const &other) variables_pattern = nullptr; // guard to prevent null dereference (always required) - if (other.init_expr != nullptr) - init_expr = other.init_expr->clone_expr (); + if (other.has_init_expr ()) + init_expr = other.get_init_expr ().clone_expr (); else init_expr = nullptr; - if (other.type != nullptr) - type = other.type->clone_type (); + if (other.has_type ()) + type = other.get_type ().clone_type (); else - type = nullptr; + type = tl::nullopt; return *this; } diff --git a/gcc/rust/hir/tree/rust-hir-stmt.h b/gcc/rust/hir/tree/rust-hir-stmt.h index 3e5adc165fba..1e17f047d3e2 100644 --- a/gcc/rust/hir/tree/rust-hir-stmt.h +++ b/gcc/rust/hir/tree/rust-hir-stmt.h @@ -22,6 +22,7 @@ #include "rust-hir.h" #include "rust-hir-path.h" #include "rust-hir-expr.h" +#include "rust-system.h" namespace Rust { namespace HIR { @@ -97,11 +98,9 @@ class LetStmt : public Stmt std::unique_ptr variables_pattern; - // bool has_type; - std::unique_ptr type; + tl::optional> type; - // bool has_init_expr; - std::unique_ptr init_expr; + tl::optional> init_expr; location_t locus; @@ -110,17 +109,18 @@ class LetStmt : public Stmt bool has_outer_attrs () const { return !outer_attrs.empty (); } // Returns whether let statement has a given return type. - bool has_type () const { return type != nullptr; } + bool has_type () const { return type.has_value (); } // Returns whether let statement has an initialisation expression. - bool has_init_expr () const { return init_expr != nullptr; } + bool has_init_expr () const { return init_expr.has_value (); } std::string as_string () const override; LetStmt (Analysis::NodeMapping mappings, std::unique_ptr variables_pattern, - std::unique_ptr init_expr, std::unique_ptr type, - AST::AttrVec outer_attrs, location_t locus); + tl::optional> init_expr, + tl::optional> type, AST::AttrVec outer_attrs, + location_t locus); // Copy constructor with clone LetStmt (LetStmt const &other); @@ -143,9 +143,29 @@ class LetStmt : public Stmt } std::vector &get_outer_attrs () { return outer_attrs; } - HIR::Type &get_type () { return *type; } + HIR::Type &get_type () + { + rust_assert (*type); + return *type.value (); + } + + const HIR::Type &get_type () const + { + rust_assert (*type); + return *type.value (); + } - HIR::Expr &get_init_expr () { return *init_expr; } + HIR::Expr &get_init_expr () + { + rust_assert (*init_expr); + return *init_expr.value (); + } + + const HIR::Expr &get_init_expr () const + { + rust_assert (*init_expr); + return *init_expr.value (); + } HIR::Pattern &get_pattern () { return *variables_pattern; } diff --git a/gcc/rust/hir/tree/rust-hir.cc b/gcc/rust/hir/tree/rust-hir.cc index f7140ad93cfe..764619115d70 100644 --- a/gcc/rust/hir/tree/rust-hir.cc +++ b/gcc/rust/hir/tree/rust-hir.cc @@ -2669,12 +2669,12 @@ LetStmt::as_string () const if (has_type ()) { - str += " : " + type->as_string (); + str += " : " + get_type ().as_string (); } if (has_init_expr ()) { - str += " = " + init_expr->as_string (); + str += " = " + get_init_expr ().as_string (); } return str; diff --git a/gcc/rust/typecheck/rust-hir-type-check-expr.cc b/gcc/rust/typecheck/rust-hir-type-check-expr.cc index 8bb5058d3fc1..d4a60c67ccec 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-expr.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-expr.cc @@ -203,14 +203,13 @@ TypeCheckExpr::visit (HIR::CallExpr &expr) ok = adt->lookup_variant_by_id (variant_id, &lookup_variant); rust_assert (ok); - variant = std::move (*lookup_variant); + variant = std::move (*lookup_variant->clone ()); } else { rust_assert (adt->number_of_variants () == 1); - variant = std::move (*adt->get_variants ().at (0)); + variant = std::move (*adt->get_variants ().at (0)->clone ()); } - infered = TyTy::TypeCheckCallExpr::go (function_tyty, expr, variant, context); return; diff --git a/gcc/rust/typecheck/rust-hir-type-check-item.cc b/gcc/rust/typecheck/rust-hir-type-check-item.cc index 12784d2650c5..17c331e9a21a 100644 --- a/gcc/rust/typecheck/rust-hir-type-check-item.cc +++ b/gcc/rust/typecheck/rust-hir-type-check-item.cc @@ -17,6 +17,7 @@ // . #include "rust-hir-type-check-item.h" +#include "optional.h" #include "rust-canonical-path.h" #include "rust-diagnostics.h" #include "rust-hir-type-check-enumitem.h" @@ -222,7 +223,7 @@ TypeCheckItem::visit (HIR::TupleStruct &struct_decl) new TyTy::VariantDef (struct_decl.get_mappings ().get_hirid (), struct_decl.get_mappings ().get_defid (), struct_decl.get_identifier ().as_string (), ident, - TyTy::VariantDef::VariantType::TUPLE, nullptr, + TyTy::VariantDef::VariantType::TUPLE, tl::nullopt, std::move (fields))); // Process #[repr(X)] attribute, if any @@ -304,7 +305,7 @@ TypeCheckItem::visit (HIR::StructStruct &struct_decl) new TyTy::VariantDef (struct_decl.get_mappings ().get_hirid (), struct_decl.get_mappings ().get_defid (), struct_decl.get_identifier ().as_string (), ident, - TyTy::VariantDef::VariantType::STRUCT, nullptr, + TyTy::VariantDef::VariantType::STRUCT, tl::nullopt, std::move (fields))); // Process #[repr(X)] attribute, if any @@ -437,7 +438,7 @@ TypeCheckItem::visit (HIR::Union &union_decl) new TyTy::VariantDef (union_decl.get_mappings ().get_hirid (), union_decl.get_mappings ().get_defid (), union_decl.get_identifier ().as_string (), ident, - TyTy::VariantDef::VariantType::STRUCT, nullptr, + TyTy::VariantDef::VariantType::STRUCT, tl::nullopt, std::move (fields))); auto *type diff --git a/gcc/rust/typecheck/rust-tyty.cc b/gcc/rust/typecheck/rust-tyty.cc index ec910b2e6740..26c061258cd2 100644 --- a/gcc/rust/typecheck/rust-tyty.cc +++ b/gcc/rust/typecheck/rust-tyty.cc @@ -18,6 +18,7 @@ #include "rust-tyty.h" +#include "optional.h" #include "rust-tyty-visitor.h" #include "rust-hir-map.h" #include "rust-location.h" @@ -1343,7 +1344,7 @@ VariantDef::variant_type_string (VariantType type) VariantDef::VariantDef (HirId id, DefId defid, std::string identifier, RustIdent ident, - std::unique_ptr &&discriminant) + tl::optional> &&discriminant) : id (id), defid (defid), identifier (identifier), ident (ident), discriminant (std::move (discriminant)) @@ -1354,7 +1355,7 @@ VariantDef::VariantDef (HirId id, DefId defid, std::string identifier, VariantDef::VariantDef (HirId id, DefId defid, std::string identifier, RustIdent ident, VariantType type, - std::unique_ptr &&discriminant, + tl::optional> &&discriminant, std::vector fields) : id (id), defid (defid), identifier (identifier), ident (ident), type (type), discriminant (std::move (discriminant)), fields (fields) @@ -1369,7 +1370,7 @@ VariantDef::get_error_node () static VariantDef node = VariantDef (UNKNOWN_HIRID, UNKNOWN_DEFID, "", {Resolver::CanonicalPath::create_empty (), UNKNOWN_LOCATION}, - nullptr); + tl::nullopt); return node; } @@ -1461,15 +1462,28 @@ VariantDef::lookup_field (const std::string &lookup, HIR::Expr & VariantDef::get_discriminant () { - rust_assert (discriminant != nullptr); - return *discriminant; + return *discriminant.value (); +} + +const HIR::Expr & +VariantDef::get_discriminant () const +{ + return *discriminant.value (); +} + +bool +VariantDef::has_discriminant () const +{ + return discriminant.has_value (); } std::string VariantDef::as_string () const { if (type == VariantType::NUM) - return identifier + " = " + discriminant->as_string (); + return identifier + + (has_discriminant () ? " = " + get_discriminant ().as_string () + : ""); std::string buffer; for (size_t i = 0; i < fields.size (); ++i) @@ -1516,8 +1530,13 @@ VariantDef::clone () const for (auto &f : fields) cloned_fields.push_back ((StructFieldType *) f->clone ()); + auto &&discriminant_opt = has_discriminant () + ? tl::optional> ( + get_discriminant ().clone_expr ()) + : tl::nullopt; + return new VariantDef (id, defid, identifier, ident, type, - discriminant->clone_expr (), cloned_fields); + std::move (discriminant_opt), cloned_fields); } VariantDef * @@ -1527,8 +1546,13 @@ VariantDef::monomorphized_clone () const for (auto &f : fields) cloned_fields.push_back ((StructFieldType *) f->monomorphized_clone ()); + auto discriminant_opt = has_discriminant () + ? tl::optional> ( + get_discriminant ().clone_expr ()) + : tl::nullopt; + return new VariantDef (id, defid, identifier, ident, type, - discriminant->clone_expr (), cloned_fields); + std::move (discriminant_opt), cloned_fields); } const RustIdent & diff --git a/gcc/rust/typecheck/rust-tyty.h b/gcc/rust/typecheck/rust-tyty.h index 2a8786169821..c896d1e03b75 100644 --- a/gcc/rust/typecheck/rust-tyty.h +++ b/gcc/rust/typecheck/rust-tyty.h @@ -570,10 +570,11 @@ class VariantDef static std::string variant_type_string (VariantType type); VariantDef (HirId id, DefId defid, std::string identifier, RustIdent ident, - std::unique_ptr &&discriminant); + tl::optional> &&discriminant); VariantDef (HirId id, DefId defid, std::string identifier, RustIdent ident, - VariantType type, std::unique_ptr &&discriminant, + VariantType type, + tl::optional> &&discriminant, std::vector fields); static VariantDef &get_error_node (); @@ -596,7 +597,10 @@ class VariantDef bool lookup_field (const std::string &lookup, StructFieldType **field_lookup, size_t *index) const; + bool has_discriminant () const; + HIR::Expr &get_discriminant (); + const HIR::Expr &get_discriminant () const; std::string as_string () const; @@ -614,8 +618,10 @@ class VariantDef std::string identifier; RustIdent ident; VariantType type; + // can either be a structure or a discriminant value - std::unique_ptr discriminant; + tl::optional> discriminant; + std::vector fields; };