Skip to content

Nr2 fix privacy path #3770

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

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gcc/rust/ast/rust-ast-collector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ TokenCollector::visit (Visibility &vis)
push (Rust::Token::make (PUB, vis.get_locus ()));
push (Rust::Token::make (LEFT_PAREN, UNDEF_LOCATION));
push (Rust::Token::make (IN, UNDEF_LOCATION));
visit (vis.get_path ());
visit (vis.get_path_unchecked ());
push (Rust::Token::make (RIGHT_PAREN, UNDEF_LOCATION));
break;
case Visibility::PRIV:
Expand Down
3 changes: 2 additions & 1 deletion gcc/rust/ast/rust-ast-visitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,8 @@ DefaultASTVisitor::visit (AST::TypeBoundWhereClauseItem &item)
void
DefaultASTVisitor::visit (AST::Visibility &vis)
{
visit (vis.get_path ());
if (vis.has_path ())
visit (vis.get_path_unchecked ());
}

void
Expand Down
3 changes: 2 additions & 1 deletion gcc/rust/ast/rust-ast.cc
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,8 @@ Visibility::as_string () const
case PUB_SUPER:
return std::string ("pub(super)");
case PUB_IN_PATH:
return std::string ("pub(in ") + in_path.as_string () + std::string (")");
return std::string ("pub(in ") + get_path_unchecked ().as_string ()
+ std::string (")");
default:
rust_unreachable ();
}
Expand Down
30 changes: 10 additions & 20 deletions gcc/rust/ast/rust-ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -497,41 +497,31 @@ struct Visibility
private:
VisType vis_type;
// Only assigned if vis_type is IN_PATH
SimplePath in_path;
tl::optional<SimplePath> in_path;
location_t locus;

// should this store location info?

public:
// Creates a Visibility - TODO make constructor protected or private?
Visibility (VisType vis_type, SimplePath in_path, location_t locus)
Visibility (VisType vis_type, tl::optional<SimplePath> in_path,
location_t locus)
: vis_type (vis_type), in_path (std::move (in_path)), locus (locus)
{}

public:
VisType get_vis_type () const { return vis_type; }

// Returns whether visibility is in an error state.
bool is_error () const
// Returns whether a visibility has a path
bool has_path () const
{
return vis_type == PUB_IN_PATH && in_path.is_empty ();
return in_path.has_value () && vis_type >= PUB_CRATE;
}

// Returns whether a visibility has a path
bool has_path () const { return !is_error () && vis_type >= PUB_CRATE; }

// Returns whether visibility is public or not.
bool is_public () const { return vis_type != PRIV && !is_error (); }
bool is_public () const { return vis_type != PRIV; }

location_t get_locus () const { return locus; }

// empty?
// Creates an error visibility.
static Visibility create_error ()
{
return Visibility (PUB_IN_PATH, SimplePath::create_empty (),
UNDEF_LOCATION);
}

// Unique pointer custom clone function
/*std::unique_ptr<Visibility> clone_visibility() const {
return std::unique_ptr<Visibility>(clone_visibility_impl());
Expand Down Expand Up @@ -591,8 +581,8 @@ struct Visibility
}

std::string as_string () const;
const SimplePath &get_path () const { return in_path; }
SimplePath &get_path () { return in_path; }
const SimplePath &get_path_unchecked () const { return in_path.value (); }
SimplePath &get_path_unchecked () { return in_path.value (); }

protected:
// Clone function implementation - not currently virtual but may be if
Expand Down
16 changes: 8 additions & 8 deletions gcc/rust/ast/rust-item.h
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ class Module : public VisItem
// Loaded module constructor, with items
Module (Identifier name, location_t locus,
std::vector<std::unique_ptr<Item>> items,
Visibility visibility = Visibility::create_error (),
Visibility visibility = Visibility::create_private (),
Unsafety safety = Unsafety::Normal,
std::vector<Attribute> inner_attrs = std::vector<Attribute> (),
std::vector<Attribute> outer_attrs = std::vector<Attribute> ())
Expand Down Expand Up @@ -1727,7 +1727,7 @@ class StructField
bool has_outer_attributes () const { return !outer_attrs.empty (); }

// Returns whether struct field has a non-private (non-default) visibility.
bool has_visibility () const { return !visibility.is_error (); }
bool has_visibility () const { return visibility.is_public (); }

StructField (Identifier field_name, std::unique_ptr<Type> field_type,
Visibility vis, location_t locus,
Expand Down Expand Up @@ -1781,8 +1781,8 @@ class StructField
// Creates an error state struct field.
static StructField create_error ()
{
return StructField (std::string (""), nullptr, Visibility::create_error (),
UNDEF_LOCATION);
return StructField (std::string (""), nullptr,
Visibility::create_private (), UNDEF_LOCATION);
}

std::string as_string () const;
Expand Down Expand Up @@ -1886,7 +1886,7 @@ class TupleField

/* Returns whether tuple field has a non-default visibility (i.e. a public
* one) */
bool has_visibility () const { return !visibility.is_error (); }
bool has_visibility () const { return visibility.is_public (); }

// Complete constructor
TupleField (std::unique_ptr<Type> field_type, Visibility vis,
Expand Down Expand Up @@ -1936,7 +1936,7 @@ class TupleField
// Creates an error state tuple field.
static TupleField create_error ()
{
return TupleField (nullptr, Visibility::create_error (), UNDEF_LOCATION);
return TupleField (nullptr, Visibility::create_private (), UNDEF_LOCATION);
}

std::string as_string () const;
Expand Down Expand Up @@ -3466,7 +3466,7 @@ class ExternalTypeItem : public ExternalItem
bool has_outer_attrs () const { return !outer_attrs.empty (); }

// Returns whether item has non-default visibility.
bool has_visibility () const { return !visibility.is_error (); }
bool has_visibility () const { return visibility.is_public (); }

location_t get_locus () const { return locus; }

Expand Down Expand Up @@ -3558,7 +3558,7 @@ class ExternalStaticItem : public ExternalItem
bool has_outer_attrs () const { return !outer_attrs.empty (); }

// Returns whether item has non-default visibility.
bool has_visibility () const { return !visibility.is_error (); }
bool has_visibility () const { return visibility.is_public (); }

location_t get_locus () const { return locus; }

Expand Down
2 changes: 1 addition & 1 deletion gcc/rust/ast/rust-macro.h
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ class MacroRulesDefinition : public VisItem
return std::make_unique<MacroRulesDefinition> (
MacroRulesDefinition (rule_name, delim_type, rules, outer_attrs, locus,
AST::MacroRulesDefinition::MacroKind::MBE,
AST::Visibility::create_error ()));
AST::Visibility::create_private ()));
}

static std::unique_ptr<MacroRulesDefinition>
Expand Down
10 changes: 2 additions & 8 deletions gcc/rust/hir/rust-ast-lower.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,6 @@ using HIR::ClosureParam;
Visibility
translate_visibility (const AST::Visibility &vis)
{
// FIXME: How do we create a private visibility here? Is it always private if
// the AST vis is an error?
// FIXME: We need to add a `create_private()` static function to the
// AST::Visibility class and use it when the vis is empty in the parser...
if (vis.is_error ())
return Visibility::create_error ();

switch (vis.get_vis_type ())
{
case AST::Visibility::PUB:
Expand All @@ -52,7 +45,8 @@ translate_visibility (const AST::Visibility &vis)
case AST::Visibility::PUB_SUPER:
case AST::Visibility::PUB_IN_PATH:
return Visibility (Visibility::VisType::RESTRICTED,
ASTLoweringSimplePath::translate (vis.get_path ()),
ASTLoweringSimplePath::translate (
vis.get_path_unchecked ()),
vis.get_locus ());
break;
}
Expand Down
4 changes: 2 additions & 2 deletions gcc/rust/parse/rust-parse-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2414,7 +2414,7 @@ Parser<ManagedTokenSource>::parse_visibility ()
add_error (std::move (error));

// skip after somewhere?
return AST::Visibility::create_error ();
return AST::Visibility::create_private ();
}

skip_token (RIGHT_PAREN);
Expand All @@ -2426,7 +2426,7 @@ Parser<ManagedTokenSource>::parse_visibility ()
t->get_token_description ()));

lexer.skip_token ();
return AST::Visibility::create_error ();
return AST::Visibility::create_private ();
}
}

Expand Down
2 changes: 1 addition & 1 deletion gcc/rust/resolve/rust-ast-resolve-base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ ResolverBase::resolve_visibility (const AST::Visibility &vis)
{
if (vis.has_path ())
{
auto path = vis.get_path ();
auto path = vis.get_path_unchecked ();
ResolvePath::go (path);

// Do we need to lookup something here?
Expand Down
6 changes: 5 additions & 1 deletion gcc/rust/resolve/rust-early-name-resolver-2.0.cc
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,11 @@ Early::visit (AST::MacroInvocation &invoc)
// we won't have changed `definition` from `nullopt` if there are more
// than one segments in our path
if (!definition.has_value ())
definition = ctx.resolve_path (path.get_segments (), Namespace::Macros);
{
if (auto resolved
= ctx.resolve_path (path.get_segments (), Namespace::Macros))
definition = resolved.value ();
}

// if the definition still does not have a value, then it's an error
if (!definition.has_value ())
Expand Down
23 changes: 20 additions & 3 deletions gcc/rust/resolve/rust-forever-stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,23 @@ class ForeverStackStore
Node root;
};

class ResolutionError
{
location_t offending_location;
std::vector<Identifier> suggestions;

ResolutionError (location_t loc, std::vector<Identifier> suggestions)
: offending_location (loc), suggestions (suggestions)
{}

public:
static ResolutionError make_error (location_t loc,
std::vector<Identifier> suggestions = {})
{
return ResolutionError (loc, suggestions);
}
};

template <Namespace N> class ForeverStack
{
public:
Expand Down Expand Up @@ -671,7 +688,7 @@ template <Namespace N> class ForeverStack
* current map, an empty one otherwise.
*/
template <typename S>
tl::optional<Rib::Definition> resolve_path (
tl::expected<Rib::Definition, ResolutionError> resolve_path (
const std::vector<S> &segments, bool has_opening_scope_resolution,
std::function<void (const S &, NodeId)> insert_segment_resolution,
std::vector<Error> &collect_errors);
Expand Down Expand Up @@ -790,14 +807,14 @@ template <Namespace N> class ForeverStack
Node &find_closest_module (Node &starting_point);

template <typename S>
tl::optional<SegIterator<S>> find_starting_point (
tl::expected<SegIterator<S>, ResolutionError> find_starting_point (
const std::vector<S> &segments,
std::reference_wrapper<Node> &starting_point,
std::function<void (const S &, NodeId)> insert_segment_resolution,
std::vector<Error> &collect_errors);

template <typename S>
tl::optional<Node &> resolve_segments (
tl::expected<std::reference_wrapper<Node>, ResolutionError> resolve_segments (
Node &starting_point, const std::vector<S> &segments,
SegIterator<S> iterator,
std::function<void (const S &, NodeId)> insert_segment_resolution,
Expand Down
Loading
Loading