Skip to content
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

Fix bug in type resolution of paths #3277

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
13 changes: 7 additions & 6 deletions gcc/rust/resolve/rust-early-name-resolver-2.0.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ Early::go (AST::Crate &crate)
bool
Early::resolve_glob_import (NodeId use_dec_id, TopLevel::ImportKind &&glob)
{
auto resolved = ctx.types.resolve_path (glob.to_resolve.get_segments ());
auto resolved
= ctx.resolve_path (glob.to_resolve.get_segments (), Namespace::Types);
if (!resolved.has_value ())
return false;

Expand Down Expand Up @@ -255,7 +256,7 @@ 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.macros.resolve_path (path.get_segments ());
definition = ctx.resolve_path (path.get_segments (), Namespace::Macros);

// if the definition still does not have a value, then it's an error
if (!definition.has_value ())
Expand Down Expand Up @@ -296,8 +297,8 @@ Early::visit_attributes (std::vector<AST::Attribute> &attrs)
auto traits = attr.get_traits_to_derive ();
for (auto &trait : traits)
{
auto definition
= ctx.macros.resolve_path (trait.get ().get_segments ());
auto definition = ctx.resolve_path (trait.get ().get_segments (),
Namespace::Macros);
if (!definition.has_value ())
{
// FIXME: Change to proper error message
Expand All @@ -320,8 +321,8 @@ Early::visit_attributes (std::vector<AST::Attribute> &attrs)
->lookup_builtin (name)
.is_error ()) // Do not resolve builtins
{
auto definition
= ctx.macros.resolve_path (attr.get_path ().get_segments ());
auto definition = ctx.resolve_path (attr.get_path ().get_segments (),
Namespace::Macros);
if (!definition.has_value ())
{
// FIXME: Change to proper error message
Expand Down
9 changes: 6 additions & 3 deletions gcc/rust/resolve/rust-early-name-resolver-2.0.h
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,12 @@ class Early : public DefaultResolver
};
};

ctx.values.resolve_path (segments).map (pair_with_ns (Namespace::Values));
ctx.types.resolve_path (segments).map (pair_with_ns (Namespace::Types));
ctx.macros.resolve_path (segments).map (pair_with_ns (Namespace::Macros));
ctx.resolve_path (segments, Namespace::Values)
.map (pair_with_ns (Namespace::Values));
ctx.resolve_path (segments, Namespace::Types)
.map (pair_with_ns (Namespace::Types));
ctx.resolve_path (segments, Namespace::Macros)
.map (pair_with_ns (Namespace::Macros));

return resolved;
}
Expand Down
18 changes: 11 additions & 7 deletions gcc/rust/resolve/rust-forever-stack.h
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,9 @@ template <Namespace N> class ForeverStack
* current map, an empty one otherwise.
*/
template <typename S>
tl::optional<Rib::Definition> resolve_path (const std::vector<S> &segments);
tl::optional<Rib::Definition> resolve_path (
const std::vector<S> &segments,
std::function<void (const S &, NodeId)> insert_segment_resolution);

// FIXME: Documentation
tl::optional<Resolver::CanonicalPath> to_canonical_path (NodeId id) const;
Expand Down Expand Up @@ -764,14 +766,16 @@ template <Namespace N> class ForeverStack
Node &find_closest_module (Node &starting_point);

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

template <typename S>
tl::optional<Node &> resolve_segments (Node &starting_point,
const std::vector<S> &segments,
SegIterator<S> iterator);
tl::optional<Node &> resolve_segments (
Node &starting_point, const std::vector<S> &segments,
SegIterator<S> iterator,
std::function<void (const S &, NodeId)> insert_segment_resolution);

/* Helper functions for forward resolution (to_canonical_path, to_rib...) */
struct DfsResult
Expand Down
44 changes: 34 additions & 10 deletions gcc/rust/resolve/rust-forever-stack.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,8 @@ template <Namespace N>
template <typename S>
tl::optional<typename std::vector<S>::const_iterator>
ForeverStack<N>::find_starting_point (
const std::vector<S> &segments, std::reference_wrapper<Node> &starting_point)
const std::vector<S> &segments, std::reference_wrapper<Node> &starting_point,
std::function<void (const S &, NodeId)> insert_segment_resolution)
{
auto iterator = segments.begin ();

Expand All @@ -401,12 +402,19 @@ ForeverStack<N>::find_starting_point (
if (seg.is_crate_path_seg ())
{
starting_point = root;
// TODO: is this how we should be getting the crate node id?
auto &mappings = Analysis::Mappings::get ();
NodeId current_crate
= *mappings.crate_num_to_nodeid (mappings.get_current_crate ());

insert_segment_resolution (seg, current_crate);
iterator++;
break;
}
if (seg.is_lower_self_seg ())
{
// do nothing and exit
// insert segment resolution and exit
insert_segment_resolution (seg, starting_point.get ().id);
iterator++;
break;
}
Expand All @@ -421,6 +429,8 @@ ForeverStack<N>::find_starting_point (

starting_point
= find_closest_module (starting_point.get ().parent.value ());

insert_segment_resolution (seg, starting_point.get ().id);
continue;
}

Expand All @@ -438,7 +448,8 @@ template <typename S>
tl::optional<typename ForeverStack<N>::Node &>
ForeverStack<N>::resolve_segments (
Node &starting_point, const std::vector<S> &segments,
typename std::vector<S>::const_iterator iterator)
typename std::vector<S>::const_iterator iterator,
std::function<void (const S &, NodeId)> insert_segment_resolution)
{
auto *current_node = &starting_point;
for (; !is_last (iterator, segments); iterator++)
Expand Down Expand Up @@ -479,6 +490,7 @@ ForeverStack<N>::resolve_segments (
}

current_node = &child.value ();
insert_segment_resolution (seg, current_node->id);
}

return *current_node;
Expand All @@ -487,23 +499,35 @@ ForeverStack<N>::resolve_segments (
template <Namespace N>
template <typename S>
tl::optional<Rib::Definition>
ForeverStack<N>::resolve_path (const std::vector<S> &segments)
ForeverStack<N>::resolve_path (
const std::vector<S> &segments,
std::function<void (const S &, NodeId)> insert_segment_resolution)
{
// TODO: What to do if segments.empty() ?

// if there's only one segment, we just use `get`
if (segments.size () == 1)
return get (segments.back ().as_string ());
{
auto res = get (segments.back ().as_string ());
if (res && !res->is_ambiguous ())
insert_segment_resolution (segments.back (), res->get_node_id ());
return res;
}

std::reference_wrapper<Node> starting_point = cursor ();

return find_starting_point (segments, starting_point)
.and_then ([this, &segments, &starting_point] (
return find_starting_point (segments, starting_point,
insert_segment_resolution)
.and_then ([this, &segments, &starting_point, &insert_segment_resolution] (
typename std::vector<S>::const_iterator iterator) {
return resolve_segments (starting_point.get (), segments, iterator);
return resolve_segments (starting_point.get (), segments, iterator,
insert_segment_resolution);
})
.and_then ([&segments] (Node final_node) {
return final_node.rib.get (segments.back ().as_string ());
.and_then ([&segments, &insert_segment_resolution] (Node final_node) {
auto res = final_node.rib.get (segments.back ().as_string ());
if (res && !res->is_ambiguous ())
insert_segment_resolution (segments.back (), res->get_node_id ());
return res;
});
}

Expand Down
28 changes: 19 additions & 9 deletions gcc/rust/resolve/rust-late-name-resolver-2.0.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,12 @@ Late::visit (AST::PathInExpression &expr)

tl::optional<Rib::Definition> resolved = tl::nullopt;

if (auto value = ctx.values.resolve_path (expr.get_segments ()))
if (auto value = ctx.resolve_path (expr.get_segments (), Namespace::Values))
{
resolved = value;
}
else if (auto type = ctx.types.resolve_path (expr.get_segments ()))
else if (auto type
= ctx.resolve_path (expr.get_segments (), Namespace::Types))
{
resolved = type;
}
Expand Down Expand Up @@ -279,11 +280,17 @@ Late::visit (AST::TypePath &type)
auto values = ctx.types.peek ().get_values ();

if (auto resolved = ctx.types.get (str))
ctx.map_usage (Usage (type.get_node_id ()),
Definition (resolved->get_node_id ()));
{
ctx.map_usage (Usage (type.get_node_id ()),
Definition (resolved->get_node_id ()));
ctx.map_usage (Usage (type.get_segments ().back ()->get_node_id ()),
Definition (resolved->get_node_id ()));
}
else
rust_error_at (type.get_locus (), "could not resolve type path %qs",
str.c_str ());
{
rust_error_at (type.get_locus (), "could not resolve type path %qs",
str.c_str ());
}

DefaultResolver::visit (type);
}
Expand Down Expand Up @@ -311,7 +318,8 @@ Late::visit (AST::StructStruct &s)
void
Late::visit (AST::StructExprStruct &s)
{
auto resolved = ctx.types.resolve_path (s.get_struct_name ().get_segments ());
auto resolved
= ctx.resolve_path (s.get_struct_name ().get_segments (), Namespace::Types);

ctx.map_usage (Usage (s.get_struct_name ().get_node_id ()),
Definition (resolved->get_node_id ()));
Expand All @@ -320,7 +328,8 @@ Late::visit (AST::StructExprStruct &s)
void
Late::visit (AST::StructExprStructBase &s)
{
auto resolved = ctx.types.resolve_path (s.get_struct_name ().get_segments ());
auto resolved
= ctx.resolve_path (s.get_struct_name ().get_segments (), Namespace::Types);

ctx.map_usage (Usage (s.get_struct_name ().get_node_id ()),
Definition (resolved->get_node_id ()));
Expand All @@ -330,7 +339,8 @@ Late::visit (AST::StructExprStructBase &s)
void
Late::visit (AST::StructExprStructFields &s)
{
auto resolved = ctx.types.resolve_path (s.get_struct_name ().get_segments ());
auto resolved
= ctx.resolve_path (s.get_struct_name ().get_segments (), Namespace::Types);

ctx.map_usage (Usage (s.get_struct_name ().get_node_id ()),
Definition (resolved->get_node_id ()));
Expand Down
25 changes: 25 additions & 0 deletions gcc/rust/resolve/rust-name-resolution-context.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,31 @@ class NameResolutionContext

tl::optional<NodeId> lookup (NodeId usage) const;

template <typename S>
tl::optional<Rib::Definition> resolve_path (const std::vector<S> &segments,
Namespace ns)
{
std::function<void (const S &, NodeId)> insert_segment_resolution
= [this] (const S &seg, NodeId id) {
if (resolved_nodes.find (Usage (seg.get_node_id ()))
== resolved_nodes.end ())
map_usage (Usage (seg.get_node_id ()), Definition (id));
};
switch (ns)
{
case Namespace::Values:
return values.resolve_path (segments, insert_segment_resolution);
case Namespace::Types:
return types.resolve_path (segments, insert_segment_resolution);
case Namespace::Macros:
return macros.resolve_path (segments, insert_segment_resolution);
case Namespace::Labels:
return labels.resolve_path (segments, insert_segment_resolution);
default:
rust_unreachable ();
}
}

private:
/* Map of "usage" nodes which have been resolved to a "definition" node */
std::map<Usage, Definition> resolved_nodes;
Expand Down
5 changes: 3 additions & 2 deletions gcc/rust/typecheck/rust-hir-type-check-path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,9 @@ TypeCheckExpr::resolve_root_path (HIR::PathInExpression &expr, size_t *offset,
= Resolver2_0::ImmutableNameResolutionContext::get ().resolver ();

// assign the ref_node_id if we've found something
nr_ctx.lookup (expr.get_mappings ().get_nodeid ())
.map ([&ref_node_id] (NodeId resolved) { ref_node_id = resolved; });
nr_ctx.lookup (ast_node_id).map ([&ref_node_id] (NodeId resolved) {
ref_node_id = resolved;
});
}
else if (!resolver->lookup_resolved_name (ast_node_id, &ref_node_id))
resolver->lookup_resolved_type (ast_node_id, &ref_node_id);
Expand Down
5 changes: 3 additions & 2 deletions gcc/rust/typecheck/rust-hir-type-check-type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,9 @@ TypeCheckType::resolve_root_path (HIR::TypePath &path, size_t *offset,
= Resolver2_0::ImmutableNameResolutionContext::get ().resolver ();

// assign the ref_node_id if we've found something
nr_ctx.lookup (path.get_mappings ().get_nodeid ())
.map ([&ref_node_id] (NodeId resolved) { ref_node_id = resolved; });
nr_ctx.lookup (ast_node_id).map ([&ref_node_id] (NodeId resolved) {
powerboat9 marked this conversation as resolved.
Show resolved Hide resolved
ref_node_id = resolved;
});
}
else if (!resolver->lookup_resolved_name (ast_node_id, &ref_node_id))
resolver->lookup_resolved_type (ast_node_id, &ref_node_id);
Expand Down
Loading