Skip to content

Commit

Permalink
Fix bug in type resolution of paths
Browse files Browse the repository at this point in the history
gcc/rust/ChangeLog:

	* resolve/rust-early-name-resolver-2.0.cc
	(Early::resolve_glob_import): Use
	NameResolutionContext::resolve_path instead of
	ForeverStack::resolve_path.
	(Early::visit): Likewise.
	(Early::visit_attributes): Likewise.
	* resolve/rust-early-name-resolver-2.0.h
	(Early::resolve_path_in_all_ns): Likewise.
	* resolve/rust-late-name-resolver-2.0.cc
	(Late::visit): Likewise and insert segment resolutions not
	handled by NameResolutionContext::resolve_path.
	* resolve/rust-name-resolution-context.h
	(NameResolutionContext::resolve_path): Add.
	* typecheck/rust-hir-type-check-path.cc
	(TypeCheckExpr::resolve_root_path): Use segment node ids instead
	of the path node id to look up segment resolutions when using
	the 2.0 resolver, as is done with the 1.0 resolver.
	* typecheck/rust-hir-type-check-type.cc
	(TypeCheckType::resolve_root_path): Likewise.
	* resolve/rust-forever-stack.h
	(ForeverStack::resolve_path): Add callback parameter for
	inserting segment resolutions.
	(ForeverStack::find_starting_point): Likewise.
	(ForeverStack::resolve_segments): Likewise.
	* resolve/rust-forever-stack.hxx
	(ForeverStack::find_starting_point): Likewise.
	(ForeverStack::resolve_segments): Likewise.
	(ForeverStack::resolve_path): Likewise.

Signed-off-by: Owen Avery <[email protected]>
  • Loading branch information
powerboat9 committed Jan 11, 2025
1 parent 5db9ab5 commit 385a5ce
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 39 deletions.
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) {
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

0 comments on commit 385a5ce

Please sign in to comment.