Skip to content

Commit

Permalink
Replace reference to unique pointer with reference
Browse files Browse the repository at this point in the history
Reference to unique pointers are a known anti pattern, only the element
shall be taken by reference instead of the whole wrapper.

gcc/rust/ChangeLog:

	* ast/rust-item.h: Change getter function prototype to return a
	reference directly instead of a reference to the wrapper type.
	* checks/errors/rust-ast-validation.cc (ASTValidation::visit): Fix
	the code to accept references instead.
	* hir/rust-ast-lower-base.cc (ASTLoweringBase::lower_self): Change
	function implementation to return a reference.
	* hir/rust-ast-lower-base.h: Accept a reference instead of a unique
	pointer reference.
	* resolve/rust-ast-resolve-item.cc (ResolveItem::visit): Adapt the code
	to a reference instead of a unique pointer.

Signed-off-by: Pierre-Emmanuel Patry <[email protected]>
  • Loading branch information
P-E-P committed Mar 13, 2024
1 parent 444d1a9 commit 843d7d7
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 28 deletions.
8 changes: 4 additions & 4 deletions gcc/rust/ast/rust-item.h
Original file line number Diff line number Diff line change
Expand Up @@ -1415,15 +1415,15 @@ class Function : public VisItem, public AssociatedItem, public ExternalItem
return return_type;
}

std::unique_ptr<Param> &get_self_param ()
Param &get_self_param ()
{
rust_assert (has_self_param ());
return function_params[0];
return *function_params[0];
}
const std::unique_ptr<Param> &get_self_param () const
const Param &get_self_param () const
{
rust_assert (has_self_param ());
return function_params[0];
return *function_params[0];
}

// ExternalItem::node_id is same as Stmt::node_id
Expand Down
2 changes: 1 addition & 1 deletion gcc/rust/checks/errors/rust-ast-validation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ ASTValidation::visit (AST::Function &function)
&& context.back () != Context::INHERENT_IMPL
&& function.has_self_param ())
rust_error_at (
function.get_self_param ()->get_locus (),
function.get_self_param ().get_locus (),
"%<self%> parameter is only allowed in associated functions");

if (function.is_external ())
Expand Down
24 changes: 12 additions & 12 deletions gcc/rust/hir/rust-ast-lower-base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -648,31 +648,31 @@ ASTLoweringBase::lower_generic_args (AST::GenericArgs &args)
}

HIR::SelfParam
ASTLoweringBase::lower_self (std::unique_ptr<AST::Param> &param)
ASTLoweringBase::lower_self (AST::Param &param)
{
rust_assert (param->is_self ());
rust_assert (param.is_self ());

auto self = static_cast<AST::SelfParam *> (param.get ());
auto self = static_cast<AST::SelfParam &> (param);
auto crate_num = mappings->get_current_crate ();
Analysis::NodeMapping mapping (crate_num, self->get_node_id (),
Analysis::NodeMapping mapping (crate_num, self.get_node_id (),
mappings->get_next_hir_id (crate_num),
mappings->get_next_localdef_id (crate_num));

if (self->has_type ())
if (self.has_type ())
{
HIR::Type *type = ASTLoweringType::translate (self->get_type ().get ());
HIR::Type *type = ASTLoweringType::translate (self.get_type ().get ());
return HIR::SelfParam (mapping, std::unique_ptr<HIR::Type> (type),
self->get_is_mut (), self->get_locus ());
self.get_is_mut (), self.get_locus ());
}
else if (!self->get_has_ref ())
else if (!self.get_has_ref ())
{
return HIR::SelfParam (mapping, std::unique_ptr<HIR::Type> (nullptr),
self->get_is_mut (), self->get_locus ());
self.get_is_mut (), self.get_locus ());
}

AST::Lifetime l = self->get_lifetime ();
return HIR::SelfParam (mapping, lower_lifetime (l), self->get_is_mut (),
self->get_locus ());
AST::Lifetime l = self.get_lifetime ();
return HIR::SelfParam (mapping, lower_lifetime (l), self.get_is_mut (),
self.get_locus ());
}

HIR::Type *
Expand Down
2 changes: 1 addition & 1 deletion gcc/rust/hir/rust-ast-lower-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ class ASTLoweringBase : public AST::ASTVisitor

HIR::GenericArgsBinding lower_binding (AST::GenericArgsBinding &binding);

HIR::SelfParam lower_self (std::unique_ptr<AST::Param> &self);
HIR::SelfParam lower_self (AST::Param &self);

HIR::Type *lower_type_no_bounds (AST::TypeNoBounds *type);

Expand Down
19 changes: 9 additions & 10 deletions gcc/rust/resolve/rust-ast-resolve-item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -489,32 +489,31 @@ ResolveItem::visit (AST::Function &function)
if (function.has_self_param ())
{
// self turns into (self: Self) as a function param
std::unique_ptr<AST::Param> &s_param = function.get_self_param ();
auto self_param = static_cast<AST::SelfParam *> (s_param.get ());
AST::Param &s_param = function.get_self_param ();
auto &self_param = static_cast<AST::SelfParam &> (s_param);

// FIXME: which location should be used for Rust::Identifier `self`?
AST::IdentifierPattern self_pattern (
self_param->get_node_id (), {"self"}, self_param->get_locus (),
self_param->get_has_ref (), self_param->get_is_mut (),
self_param.get_node_id (), {"self"}, self_param.get_locus (),
self_param.get_has_ref (), self_param.get_is_mut (),
std::unique_ptr<AST::Pattern> (nullptr));
PatternDeclaration::go (&self_pattern, Rib::ItemType::Param);

if (self_param->has_type ())
if (self_param.has_type ())
{
// This shouldn't happen the parser should already error for this
rust_assert (!self_param->get_has_ref ());
ResolveType::go (self_param->get_type ().get ());
rust_assert (!self_param.get_has_ref ());
ResolveType::go (self_param.get_type ().get ());
}
else
{
// here we implicitly make self have a type path of Self
std::vector<std::unique_ptr<AST::TypePathSegment>> segments;
segments.push_back (std::unique_ptr<AST::TypePathSegment> (
new AST::TypePathSegment ("Self", false,
self_param->get_locus ())));
new AST::TypePathSegment ("Self", false, self_param.get_locus ())));

AST::TypePath self_type_path (std::move (segments),
self_param->get_locus ());
self_param.get_locus ());
ResolveType::go (&self_type_path);
}
}
Expand Down

0 comments on commit 843d7d7

Please sign in to comment.