-
Notifications
You must be signed in to change notification settings - Fork 163
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
removed EnumItemDiscriminant and associated functions #3355
base: master
Are you sure you want to change the base?
Changes from all commits
0ca4ff4
54f417a
db53b76
ea9ed37
057c7e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2006,6 +2006,8 @@ class EnumItem : public VisItem | |
|
||
location_t locus; | ||
|
||
std::unique_ptr<Expr> expression; | ||
|
||
public: | ||
enum class Kind | ||
{ | ||
|
@@ -2039,7 +2041,6 @@ class EnumItem : public VisItem | |
// | ||
// gccrs#3340 | ||
|
||
Discriminant, | ||
}; | ||
|
||
virtual ~EnumItem () {} | ||
|
@@ -2050,6 +2051,19 @@ class EnumItem : public VisItem | |
variant_name (std::move (variant_name)), locus (locus) | ||
{} | ||
|
||
EnumItem (Identifier variant_name, Visibility vis, | ||
std::vector<Attribute> outer_attrs, location_t locus, | ||
std::unique_ptr<AST::Expr> Discriminant) | ||
: VisItem (std::move (vis), std::move (outer_attrs)), | ||
variant_name (std::move (variant_name)), locus (locus), | ||
expression (Discriminant->clone_expr ()) | ||
{} | ||
|
||
EnumItem (EnumItem const &other) | ||
: VisItem (other), variant_name (other.variant_name), locus (other.locus), | ||
expression (other.expression->clone_expr ()) | ||
{} | ||
|
||
Comment on lines
+2054
to
+2066
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove these constructors |
||
virtual Kind get_enum_item_kind () const { return Kind::Identifier; } | ||
|
||
// Unique pointer custom clone function | ||
|
@@ -2073,6 +2087,20 @@ class EnumItem : public VisItem | |
|
||
Item::Kind get_item_kind () const override { return Item::Kind::EnumItem; } | ||
|
||
bool has_expr () { return expression != nullptr; } | ||
|
||
Expr &get_expr () | ||
{ | ||
rust_assert (expression != nullptr); | ||
return *expression; | ||
} | ||
|
||
std::unique_ptr<Expr> &get_expr_ptr () | ||
{ | ||
rust_assert (expression != nullptr); | ||
return expression; | ||
} | ||
|
||
Comment on lines
+2090
to
+2103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't need these as well |
||
protected: | ||
EnumItem *clone_item_impl () const override { return new EnumItem (*this); } | ||
}; | ||
|
@@ -2161,72 +2189,6 @@ class EnumItemStruct : public EnumItem | |
} | ||
}; | ||
|
||
// A discriminant (numbered enum) item used in an "enum" tagged union | ||
class EnumItemDiscriminant : public EnumItem | ||
{ | ||
std::unique_ptr<Expr> expression; | ||
|
||
public: | ||
EnumItemDiscriminant (Identifier variant_name, Visibility vis, | ||
std::unique_ptr<Expr> expr, | ||
std::vector<Attribute> outer_attrs, location_t locus) | ||
: EnumItem (std::move (variant_name), std::move (vis), | ||
std::move (outer_attrs), locus), | ||
expression (std::move (expr)) | ||
{} | ||
|
||
// Copy constructor with clone | ||
EnumItemDiscriminant (EnumItemDiscriminant const &other) | ||
: EnumItem (other), expression (other.expression->clone_expr ()) | ||
{} | ||
|
||
// Overloaded assignment operator to clone | ||
EnumItemDiscriminant &operator= (EnumItemDiscriminant const &other) | ||
{ | ||
EnumItem::operator= (other); | ||
expression = other.expression->clone_expr (); | ||
// variant_name = other.variant_name; | ||
// outer_attrs = other.outer_attrs; | ||
|
||
return *this; | ||
} | ||
|
||
// move constructors | ||
EnumItemDiscriminant (EnumItemDiscriminant &&other) = default; | ||
EnumItemDiscriminant &operator= (EnumItemDiscriminant &&other) = default; | ||
|
||
EnumItem::Kind get_enum_item_kind () const override | ||
{ | ||
return EnumItem::Kind::Discriminant; | ||
} | ||
|
||
std::string as_string () const override; | ||
|
||
void accept_vis (ASTVisitor &vis) override; | ||
|
||
bool has_expr () { return expression != nullptr; } | ||
|
||
// TODO: is this better? Or is a "vis_block" better? | ||
Expr &get_expr () | ||
{ | ||
rust_assert (expression != nullptr); | ||
return *expression; | ||
} | ||
|
||
std::unique_ptr<Expr> &get_expr_ptr () | ||
{ | ||
rust_assert (expression != nullptr); | ||
return expression; | ||
} | ||
|
||
protected: | ||
// Clone function implementation as (not pure) virtual method | ||
EnumItemDiscriminant *clone_item_impl () const override | ||
{ | ||
return new EnumItemDiscriminant (*this); | ||
} | ||
}; | ||
|
||
// AST node for Rust "enum" - tagged union | ||
class Enum : public VisItem | ||
{ | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4793,11 +4793,9 @@ Parser<ManagedTokenSource>::parse_enum_item () | |||||
|
||||||
std::unique_ptr<AST::Expr> discriminant_expr = parse_expr (); | ||||||
|
||||||
return std::unique_ptr<AST::EnumItemDiscriminant> ( | ||||||
new AST::EnumItemDiscriminant (std::move (item_name), std::move (vis), | ||||||
std::move (discriminant_expr), | ||||||
std::move (outer_attrs), | ||||||
item_name_tok->get_locus ())); | ||||||
return std::unique_ptr<AST::EnumItem> (new AST::EnumItem ( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here we need to add a comment saying that this part needs to be reworked to properly handle enum discriminants, and link to issue #3340 something like: // FIXME: We need to add special handling for parsing discriminant expressions for all possible enum items, not just enum item identifiers (gccrs#3340) |
||||||
std::move (item_name), std::move (vis), std::move (outer_attrs), | ||||||
item_name_tok->get_locus (), discriminant_expr->clone_expr ())); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to not use the
Suggested change
|
||||||
} | ||||||
default: | ||||||
// regular enum with just an identifier | ||||||
|
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.
I don't think we want to add handling for this just yet because we're not handling it properly and parsing it for all enum items yet. So I would say remove this