Prepare lang-item `{AST, HIR}::PathInExpressions` #3378

CohenArthur · 2025-01-22T16:12:24Z

This is an important base for desugaring expression lang items, e.g. when calling something like IntoIterator::into_iter(i). This changes our AST and HIR visitors to make it possible to use lang items in more places which will be necessary.

gcc/rust/ast/rust-ast-collector.cc

gcc/rust/ast/rust-ast-visitor.cc

gcc/rust/backend/rust-compile-resolve-path.cc

gcc/rust/checks/lints/rust-lint-marklive.cc

gcc/rust/hir/tree/rust-hir.cc

gcc/rust/resolve/rust-ast-resolve-path.cc

gcc/rust/resolve/rust-early-name-resolver.cc

CohenArthur · 2025-01-22T16:18:48Z

gcc/rust/typecheck/rust-hir-type-check-path.cc

+      // FIXME: We probably need to check *if* the type needs substitutions
+      // or not
+      if (LangItem::IsEnumVariant (expr.get_lang_item ()))
+	{
+	  std::pair<HIR::Enum *, HIR::EnumItem *> enum_item_lookup
+	    = mappings.lookup_hir_enumitem (*hir_id);
+	  bool enum_item_ok = enum_item_lookup.first != nullptr
+			      && enum_item_lookup.second != nullptr;
+	  rust_assert (enum_item_ok);
+
+	  HirId variant_id
+	    = enum_item_lookup.second->get_mappings ().get_hirid ();
+
+	  HIR::EnumItem *enum_item = enum_item_lookup.second;
+	  resolved_node_id = enum_item->get_mappings ().get_nodeid ();
+
+	  // insert the id of the variant we are resolved to
+	  context->insert_variant_definition (expr.get_mappings ().get_hirid (),
+					      variant_id);
+
+	  query_type (variant_id, &infered);
+	  infered = SubstMapper::InferSubst (infered, expr.get_locus ());
+	}


This is super dodgy, @philberty please check it good lol

If I could ask, how could you end up with a lang-item path that DOESN'T need substitution inferencing? I'm curious because I think the only way you could generate an option type that doesn't need inferencing would be to generate a lang-item path like Option::<i32>::Some(3), but that wouldn't be possible since lang-item paths wouldn't have PathExprSegments which can be generic arguments

The only other way would be to have a lang-item path to an enumeration variant with no generic parameter?

Yeah maybe something like that, but you're right that I don't think it's the case - there aren't that many enum variants long items, and I think they are all generic. Still, it feels wrong to always do substitution mapping lol so that's why I added the comment. It might also evolve in the future. I'm happy removing the comment though

I think it's probably better to keep it I would imagine, since if it ever becomes an issue, the comment clarifies that ALWAYS doing substitution inference is not a design choice

there might be a change needed for the generics but would need to try it out a bit myself to try our some tests on it

gcc/rust/util/rust-lang-item.cc

liamnaddell

LGTM

liamnaddell · 2025-01-22T18:17:10Z

gcc/rust/backend/rust-compile-resolve-path.cc

+    final_segment
+      = HIR::PathIdentSegment (LangItem::ToString (expr.get_lang_item ()));


Could this end up getting confused if there are multiple Some declarations floating around?

I.e.

fn Some () { let _ = option_env!("PWD"); // Yields lang-item path to Some("/tmp") }

Maybe - I'm adding a different implementation right now which is more precise and makes use of the lang item's known NodeId

I think the version you have up now works, at least from tracing the code by eye.

philberty

LGTM lets get this merged soon

philberty

LGTM nice work the artification ident segment will work fine here

philberty · 2025-01-28T16:46:21Z

gcc/rust/backend/rust-compile-resolve-path.cc

@@ -32,32 +32,39 @@
 namespace Rust {
 namespace Compile {

-void
-ResolvePathRef::visit (HIR::QualifiedPathInExpression &expr)
+template <typename T>


I always forget you can do this!!! thanks for reminding me of this trick :D

philberty · 2025-01-28T16:56:38Z

gcc/rust/backend/rust-compile-resolve-path.cc

+	= Analysis::Mappings::get ().get_lang_item_node (expr.get_lang_item ());
+
+      // FIXME: Is that correct? :/
+      auto final_segment


yes this should work the only reason i passed the node around was because it was handy to get the location and node id so in theory it can fall back to the query system interface.

Not part of your pr but yeah this has given me alot of though of loads of good-first-pr's to cleanup the interfaces here
its super confusing for new people.

philberty · 2025-01-28T16:57:12Z

gcc/rust/backend/rust-compile-resolve-path.cc

+
+      return resolve_with_node_id (final_segment, expr.get_mappings (),
+				   expr.get_locus (), true, lang_item);
+    }
  else


I think the only change i might do here is get rid of the else and just return that

thanks! fixed in the latest version

philberty · 2025-01-28T16:57:22Z

gcc/rust/backend/rust-compile-resolve-path.cc

-
-  resolved
-    = resolve (final_segment, expr.get_mappings (), expr.get_locus (), true);
+  resolved = resolve_path_like (expr);


i love this so much

philberty · 2025-01-28T16:59:11Z

gcc/rust/backend/rust-compile-resolve-path.cc

+ResolvePathRef::resolve_with_node_id (
+  const HIR::PathIdentSegment &final_segment,
+  const Analysis::NodeMapping &mappings, location_t expr_locus,
+  bool is_qualified_path, NodeId resolved_node_id)


I have a feeling the is_qualified_path is no longer used here now which is neat

ah I kept it cause we need it for query_compile

gcc/rust/ChangeLog: * ast/rust-collect-lang-items.h: Declare visitor. * ast/rust-collect-lang-items.cc (CollectLangItems::visit): New.

gcc/rust/ChangeLog: * util/rust-hir-map.cc (Mappings::get_lang_item_node): Better formatting when a lang item does not exist when it should.

gcc/rust/ChangeLog: * util/rust-lang-item.cc (LangItem::IsEnumVariant): New function. * util/rust-lang-item.h: Declare it.

gcc/rust/ChangeLog: * ast/rust-ast-collector.cc (TokenCollector::visit): Adapt visitor to lang item PathInExpressions. * ast/rust-ast-visitor.cc (DefaultASTVisitor::visit): Likewise. * expand/rust-cfg-strip.cc (CfgStrip::visit): Likewise. * expand/rust-expand-visitor.cc (ExpandVisitor::visit): Likewise. * hir/rust-ast-lower.cc (ASTLoweringExprWithBlock::visit): Likewise. (ASTLowerPathInExpression::visit): Likewise. * resolve/rust-ast-resolve-path.cc (ResolvePath::resolve_path): Likewise. * resolve/rust-early-name-resolver.cc (EarlyNameResolver::visit): Likewise.

gcc/rust/ChangeLog: * hir/tree/rust-hir-path.h: Adapt PathPattern to accept lang-item paths. * hir/tree/rust-hir-path.cc: Assert we are dealing with a segmented path, create lang-item constructors. * hir/tree/rust-hir.cc (PathPattern::convert_to_simple_path): Likewise.

gcc/rust/ChangeLog: * backend/rust-compile-resolve-path.cc (ResolvePathRef::visit): Adapt visitor to lang item HIR::PathInExpressions. * typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::visit): Likewise.

gcc/rust/ChangeLog: * checks/lints/rust-lint-marklive.cc (MarkLive::visit): Adapt to lang items.

gcc/rust/ChangeLog: * ast/rust-path.h: New function.

gcc/rust/ChangeLog: * backend/rust-compile-resolve-path.cc (ResolvePathRef::visit): Call into resolve_path_like instead. (ResolvePathRef::resolve_path_like): New. (ResolvePathRef::resolve): Call into resolve_with_node_id. * backend/rust-compile-resolve-path.h: Declare new functions and document them.

CohenArthur added this to the Question mark operator milestone Jan 22, 2025

CohenArthur commented Jan 22, 2025

View reviewed changes

CohenArthur force-pushed the prepare-lang-item-pathinexpressions branch 2 times, most recently from 19b0e5f to 3e31b59 Compare January 22, 2025 17:03

liamnaddell approved these changes Jan 22, 2025

View reviewed changes

liamnaddell reviewed Jan 22, 2025

View reviewed changes

CohenArthur force-pushed the prepare-lang-item-pathinexpressions branch from 3e31b59 to 70384f2 Compare January 23, 2025 12:15

philberty approved these changes Jan 23, 2025

View reviewed changes

philberty assigned CohenArthur Jan 23, 2025

CohenArthur force-pushed the prepare-lang-item-pathinexpressions branch from 281fcbf to 9053922 Compare January 28, 2025 16:01

philberty approved these changes Jan 28, 2025

View reviewed changes

CohenArthur force-pushed the prepare-lang-item-pathinexpressions branch from 9053922 to a53f5b0 Compare January 28, 2025 17:09

CohenArthur added 9 commits January 28, 2025 17:10

lang-items: Collect enum variants as lang items

53c80ce

gcc/rust/ChangeLog: * ast/rust-collect-lang-items.h: Declare visitor. * ast/rust-collect-lang-items.cc (CollectLangItems::visit): New.

mappings: Improve error message for get_lang_item_node

38f0ed2

gcc/rust/ChangeLog: * util/rust-hir-map.cc (Mappings::get_lang_item_node): Better formatting when a lang item does not exist when it should.

lang-items: Add LangItem::IsEnumVariant

3063285

gcc/rust/ChangeLog: * util/rust-lang-item.cc (LangItem::IsEnumVariant): New function. * util/rust-lang-item.h: Declare it.

hir: Adapt visitors to lang item PathInExpressions

7de7fe7

gcc/rust/ChangeLog: * backend/rust-compile-resolve-path.cc (ResolvePathRef::visit): Adapt visitor to lang item HIR::PathInExpressions. * typecheck/rust-hir-type-check-path.cc (TypeCheckExpr::visit): Likewise.

marklive: Fix handling for lang item PathInExpressions.

322cd09

gcc/rust/ChangeLog: * checks/lints/rust-lint-marklive.cc (MarkLive::visit): Adapt to lang items.

ast: Add Path::is_lang_item()

dd155ff

gcc/rust/ChangeLog: * ast/rust-path.h: New function.

CohenArthur force-pushed the prepare-lang-item-pathinexpressions branch from a53f5b0 to b85c362 Compare January 28, 2025 17:10

CohenArthur enabled auto-merge January 28, 2025 17:10

CohenArthur added this pull request to the merge queue Jan 28, 2025

Merged via the queue into Rust-GCC:master with commit 6a23462 Jan 28, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare lang-item `{AST, HIR}::PathInExpressions` #3378

Prepare lang-item `{AST, HIR}::PathInExpressions` #3378

CohenArthur commented Jan 22, 2025

CohenArthur Jan 22, 2025

liamnaddell Jan 22, 2025 •

edited

Loading

liamnaddell Jan 22, 2025

CohenArthur Jan 22, 2025

liamnaddell Jan 22, 2025

philberty Jan 23, 2025

liamnaddell left a comment

liamnaddell Jan 22, 2025 •

edited

Loading

CohenArthur Jan 24, 2025

liamnaddell Jan 24, 2025

philberty left a comment

philberty left a comment

philberty Jan 28, 2025

philberty Jan 28, 2025

philberty Jan 28, 2025

CohenArthur Jan 28, 2025

philberty Jan 28, 2025

philberty Jan 28, 2025

CohenArthur Jan 28, 2025

		final_segment
		= HIR::PathIdentSegment (LangItem::ToString (expr.get_lang_item ()));

Prepare lang-item {AST, HIR}::PathInExpressions #3378

Prepare lang-item {AST, HIR}::PathInExpressions #3378

Conversation

CohenArthur commented Jan 22, 2025

Choose a reason for hiding this comment

liamnaddell Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liamnaddell left a comment

Choose a reason for hiding this comment

liamnaddell Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philberty left a comment

Choose a reason for hiding this comment

philberty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prepare lang-item `{AST, HIR}::PathInExpressions` #3378

Prepare lang-item `{AST, HIR}::PathInExpressions` #3378

liamnaddell Jan 22, 2025 •

edited

Loading

liamnaddell Jan 22, 2025 •

edited

Loading