Allow visibility modifiers for extern types #2897
Comments
|
Can i work on this issue with some help, I don't know where I should start? |
|
@badumbatish I'll try writing a starting guide here today but I'm very busy, please ping me on Zulip if you get to working on this and I still haven't written anything |
i'll probably work on this on the weekend, no worries |
|
alright, so here's how I would go about solving this. first, we need to make sure that visibility modifiers are actually parsed for extern type nodes ( if you're sure that visibility is parsed (e.g. if the parsing function is called with a visibility, or that we call now finally, the next and last step is to make sure that the AST visibility is correctly lowered to an HIR visibility when we lower our once that's done, in a later PR/issue we'll check that the visibility modifiers are actually working for external types and not just ignored - but that's for later and concerns another part of the compiler :) |
Check-list:
(x) Make sure that visibility modifier are parsed for extern type nodes:
I made sure that `parse_external_type_item` has AST::Visibility within
its declaration
```c++
// in rust-parse-impl.h
template <typename ManagedTokenSource>
std::unique_ptr<AST::ExternalTypeItem>
Parser<ManagedTokenSource>::parse_external_type_item (AST::Visibility vis,
AST::AttrVec outer_attrs);
```
(x) Make sure that visibility is assigned to `ExternalTypeItem`
I made sure that in in the function mentioned above,
its constructor, ExternalTypeItem(), handled visibility
correctly via its parent's constructor
```c++
// In rust-item.h
ExternalTypeItem (Identifier item_name, Visibility vis,
std::vector<Attribute> outer_attrs, location_t locus)
: ExternalItem (), outer_attrs (std::move (outer_attrs)), visibility (vis),
item_name (std::move (item_name)), locus (locus), marked_for_strip (false)
{}
```
(x) Make sure that the AST visibility is correctly lowered to an HIR visibility
I added a translate_visibility call via
`HIR::Visibility vis = translate_visibility (type.get_visibility ());`
in the visit() function.
I also changed the constructor of HIR::ExternalTypeItem to accept a
visibility and change the way it delegate visibility to its parent's
constructor.
gcc/rust/ChangeLog:
* hir/rust-ast-lower-extern.h: Add translate_visiblity
* hir/tree/rust-hir-item.h: Fix constructor of ExternalTypeItem
Ran through git-clang-format, make check-rust passes as expected.
Check-list:
(x) Make sure that visibility modifier are parsed for extern type nodes:
I made sure that `parse_external_type_item` has AST::Visibility within
its declaration
```c++
// in rust-parse-impl.h
template <typename ManagedTokenSource>
std::unique_ptr<AST::ExternalTypeItem>
Parser<ManagedTokenSource>::parse_external_type_item (AST::Visibility vis,
AST::AttrVec outer_attrs);
```
(x) Make sure that visibility is assigned to `ExternalTypeItem`
I made sure that in in the function mentioned above,
its constructor, ExternalTypeItem(), handled visibility
correctly via its parent's constructor
```c++
// In rust-item.h
ExternalTypeItem (Identifier item_name, Visibility vis,
std::vector<Attribute> outer_attrs, location_t locus)
: ExternalItem (), outer_attrs (std::move (outer_attrs)), visibility (vis),
item_name (std::move (item_name)), locus (locus), marked_for_strip (false)
{}
```
(x) Make sure that the AST visibility is correctly lowered to an HIR visibility
I added a translate_visibility call via
`HIR::Visibility vis = translate_visibility (type.get_visibility ());`
in the visit() function.
I also changed the constructor of HIR::ExternalTypeItem to accept a
visibility and change the way it delegate visibility to its parent's
constructor.
gcc/rust/ChangeLog:
* hir/rust-ast-lower-extern.h: Add translate_visiblity
* hir/tree/rust-hir-item.h: Fix constructor of ExternalTypeItem
Currently we do not lower/handle the visibility modifiers properly for
ExternalTypeItemnodes.The text was updated successfully, but these errors were encountered: