-
Notifications
You must be signed in to change notification settings - Fork 179
AST: Add reconstruct_type()
method
#3799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
bbeb50b
to
5a396e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice use of a template
reconstruct_vec (const std::vector<std::unique_ptr<T>> &to_reconstruct, | ||
F method) | ||
{ | ||
std::vector<std::unique_ptr<T>> reconstructed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would make sense to reserve space in reconstructed
, since we should have to_reconstruct.size () == reconstructed.size ()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^^
TLDR, it looks like you can override functions with functions that return a different type, if you're not overriding a virtual function https://godbolt.org/z/TW1rjbMba #include <memory>
class A
{
public:
std::unique_ptr<A> clone()
{
return std::make_unique<A> ();
}
};
class B: public A
{
public:
std::unique_ptr<B> clone()
{
return std::make_unique<B> ();
}
};
int main() {
A w;
std::unique_ptr<A> x = w.clone ();
B y;
std::unique_ptr<A> z = y.clone ();
} So |
8426e95
to
caf27ba
Compare
… IDs gcc/rust/ChangeLog: * ast/rust-ast.h (reconstruct): New function for calling the `reconstruct_*_impl` method and asserting that the new NodeId is different, and then wrap it in a unique_ptr<T>. (reconstruct_vec): Likewise, but for vectors of unique_ptr<T>
gcc/rust/ChangeLog: * ast/rust-ast.h: Add reconstruct_type() and reconstruct_type_impl() * ast/rust-type.h: Implement them. * ast/rust-macro.h: Likewise. * ast/rust-path.h: Likewise.
gcc/rust/ChangeLog: * Make-lang.in: Remove object file for ASTTypeBuilder. * ast/rust-ast-builder.h: Remove function. * ast/rust-ast-builder.cc (Builder::new_type): Likewise. (Builder::new_const_param): Use reconstruct_type() instead. (Builder::new_generic_args): Likewise. * expand/rust-derive-default.cc (DeriveDefault::visit_struct): Likewise. (DeriveDefault::visit_tuple): Likewise. * expand/rust-derive-eq.cc (DeriveEq::visit_tuple): Likewise. (DeriveEq::visit_struct): Likewise. (DeriveEq::visit_enum): Likewise. (DeriveEq::visit_union): Likewise. * ast/rust-ast-builder-type.cc: Removed. * ast/rust-ast-builder-type.h: Removed.
reconstruct_vec (const std::vector<std::unique_ptr<T>> &to_reconstruct, | ||
F method) | ||
{ | ||
std::vector<std::unique_ptr<T>> reconstructed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^^^
std::vector<std::unique_ptr<T>> reconstructed; | ||
|
||
for (const auto &elt : to_reconstruct) | ||
reconstructed.emplace_back (std::unique_ptr<T> (elt->reconstruct_impl ())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably use reconstruct_base
here, for the rust_assert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't work for macro invocations, as reconstruct
returns a std::unique_ptr<Type>
but we want to store a vec<ptr<MacroInvocation>>
for the pending eager macro invocations. I think in general this is a more generic version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can declare MacroInvocation::reconstruct
returning a std::unique_ptr<MacroInvocation>
The "^^^" comment was supposed to be a reply to #3799 (comment) |
ast: reconstruct: Add base for reconstructing and asserting different IDs
ast: Add reconstruct_type() method
ast: builder: Remove ASTTypeBuilder
This is the first step of adding the
reconstruct_*()
methods as discussed on Zulip. The goal is to have something similar toclone_*()
, but each time reconstructing the node and assigning it a differentNodeID
- and modifyingclone_*()
where necessary to ensure the node ID stays the same. Subsequently, this also deletes theASTTypeBuilder
visitor as it is replaced by simple calls to.reconstruct_type()
instead.