Lower ErrorPropagationExpr from AST to HIR

[bugaevc]

I have no idea what I'm doing, but using a ? was causing an ICE and now it does not (I get regular errors instead), so, good? A step towards #166 perhaps?

[philberty]

LGTM

This adds the HIR lowering with no desugaring, but we are probably missing the name-resolution for this and type-checking then we need code-generation etc...

[philberty]

IF you fix the commit message format, then this is good to merge.

[bugaevc]

Thanks!

This adds the HIR lowering with no desugaring

Yes. I saw that there exists HIR::ErrorPropagationExpr, and understood it to mean that desugaring is expected to happen at some later stage, after the AST-to-HIR lowering. Was that the right call? At which stage should it be desugared?

I was going to look into how lang items are implemented next, to see if I could find/add try_trait / try_trait_v2 and do something about it.

IF you fix the commit message format, then this is good to merge.

I will, but that will have to wait until tomorrow.

[CohenArthur]

LGTM :) If you're interested in taking care of the later parts of the pipeline we'd be more than happy to help, if you want to chat on our Zulip to get quicker feedback on what to do etc feel free! Thank you :D

[bugaevc]

If you're interested in taking care of the later parts of the pipeline

I'd love to!

if you want to chat on our Zulip to get quicker feedback on what to do etc feel free!

Maybe, but I must admit the prospect of joining yet another IM platform does not sound very appealing 🙂 Can't we all just go back to IRC and plain text mailing lists. So while I'm undecided, would you be so kind as to give me some guidance here on GH?

• What is the compilation pipeline?
  • The source code is parsed into AST which is then lowered to HIR, that much I understand.
  • What happens next? GENERIC? GIMPLE? (Maybe you could point me to where in the source this happens, too.)
  • Is there a MIR?
• Does/should the desugaring happen during a HIR-to-HIR transform pass? Or maybe do different desugarings happen in different phases?
• What are the major differences between AST and HIR? From what I've seen so far (and done in this PR), it's mostly a 1-to-1 translation.
• What does this Analysis::NodeMapping thing even do?
  • I'm not dumb, I'm just not very familiar with GCC / gccrs internals, that's all 🙂
• For desugaring, I need to look up a trait marked with lang = "try"; how would I do that? Is there some sort of lang item -marked item registry where I could do something like registry ()->look_up (RustLangItem::TRY)?
  • I need to look into how traits for arithmetic ops are looked up and just do the same. But maybe you can just tell me the answer straight up.

[bugaevc]

• For desugaring, I need to look up a trait marked with lang = "try"; how would I do that? Is there some sort of lang item -marked item registry where I could do something like registry ()->look_up (RustLangItem::TRY)?
  • I need to look into how traits for arithmetic ops are looked up and just do the same.

I found this:

gccrs/gcc/rust/typecheck/rust-hir-type-check-expr.cc

Lines 1581 to 1595 in 74b4d6d

	bool
	TypeCheckExpr::resolve_operator_overload (
	Analysis::RustLangItem::ItemType lang_item_type, HIR::OperatorExprMeta expr,
	TyTy::BaseType *lhs, TyTy::BaseType *rhs)
	{
	// look up lang item for arithmetic type
	std::string associated_item_name
	= Analysis::RustLangItem::ToString (lang_item_type);
	DefId respective_lang_item_id = UNKNOWN_DEFID;
	bool lang_item_defined
	= mappings->lookup_lang_item (lang_item_type, &respective_lang_item_id);
	// probe for the lang-item
	if (!lang_item_defined)
	return false;

(by the way, instead of just a return false this should probably emit a warning saying it could not find the required lang item?)

But also, the following looks just wrong:

gccrs/gcc/rust/typecheck/rust-hir-type-check-expr.cc

Lines 1597 to 1616 in 74b4d6d

	auto segment = HIR::PathIdentSegment (associated_item_name);
	auto candidates
	= MethodResolver::Probe (lhs, HIR::PathIdentSegment (associated_item_name));
	bool have_implementation_for_lang_item = candidates.size () > 0;
	if (!have_implementation_for_lang_item)
	return false;
	if (candidates.size () > 1)
	{
	// mutliple candidates
	RichLocation r (expr.get_locus ());
	for (auto &c : candidates)
	r.add_range (c.candidate.locus);
	rust_error_at (
	r, "multiple candidates found for possible operator overload");
	return false;
	}

We should not attempt to resolve a method named, say, add, but instead solely look at whether or not the receiver implements the one trait marked with the lang item. And indeed, testing the following:

#[lang = "add"]
pub trait Add<Rhs = Self> {
    type Output;
    fn add(self, rhs: Rhs) -> Self::Output;
}

trait AlsoAdd {
    fn add(self);
}

struct MyStruct(i32);

impl Add for MyStruct {
    type Output = i32;
    fn add(self, rhs: MyStruct) -> i32 {
        self.0 + rhs.0
    }
}

impl AlsoAdd for MyStruct {
    fn add(self) { }
}

fn foo() ->i32 {
    MyStruct(35) + MyStruct(42)
}

I get the error:

bad-add.rs:25:5: error: multiple candidates found for possible operator overload
   15 |     fn add(self, rhs: MyStruct) -> i32 {
      |     ~~
......
   21 |     fn add(self) { }
      |     ~~
......
   25 |     MyStruct(35) + MyStruct(42)
      |     ^~~~~~~~
bad-add.rs:25:5: error: cannot apply this operator to types MyStruct{MyStruct (0:i32)} and MyStruct{MyStruct (0:i32)}
bad-add.rs:25:5: error: failed to type resolve expression

Who cares whether or not there happens to be another method named add on my type! I'm not invoking .add(), I'm using +, the Add trait. rustc playground

[CohenArthur]

If you're interested in taking care of the later parts of the pipeline

I'd love to!

if you want to chat on our Zulip to get quicker feedback on what to do etc feel free!

Maybe, but I must admit the prospect of joining yet another IM platform does not sound very appealing slightly_smiling_face Can't we all just go back to IRC and plain text mailing lists.

I'll address all of the other points you raised in your message shortly, but I just wanted to say we also have an IRC channel and mailing list if you do prefer that :D #gccrust on irc.oftc.net and https://gcc.gnu.org/mailman/listinfo/gcc-rust. So if you'd rather chat on there, feel free!

[bugaevc]

but I just wanted to say we also have an IRC channel and mailing list if you do prefer that :D #gccrust on irc.oftc.net and https://gcc.gnu.org/mailman/listinfo/gcc-rust. So if you'd rather chat on there, feel free!

I was not being very serious, but thank you! 😄

The next round of questions:

• What is TraitReference, and how's it different from HIR::Trait? When would I want to use one vs the other?

• How do I programmatically do <Type as Trait>::method(receiver)? I see that for programmatically doing receiver.method(), there is MethodResolver::Probe (), but that contains a non-trivial amount of logic.

• How are generic vs concrete items represented? How do I properly work with a "somewhat specialized" item, by which I mean something like the following:

  struct SomeItem<T>;
  
  trait Foo {
      type Bar;
  }
  
  fn my_func<U: Foo>() {
      SomeItem<U::Bar>;
  }

  Here, SomeItem<U::Bar> is not the generic SomeItem<>, but a seemingly concrete, monomorphized type, except not really, because the type parameter we instantiate it with is not a specific type like i32, but rather itself dependent on whatever U is. Also do you happen to know the proper terminology to talk about this?

[CohenArthur]

• What is the compilation pipeline?
  • The source code is parsed into AST which is then lowered to HIR, that much I understand.

That's correct :)

* What happens next? GENERIC? GIMPLE? (Maybe you could point me to where in the source this happens, too.)

Our HIR is then translated to GCC's highest level intermediate representation, GENERIC (or TREE which is the same). That happens in all the visitors within gcc/rust/backend/, like rust-compile-expr.cc and so on.

* Is there a MIR?

There isn't :) we don't have the need for one at the moment.

• Does/should the desugaring happen during a HIR-to-HIR transform pass? Or maybe do different desugarings happen in different phases?

Most of the desugaring will happen during the AST-lowering pass. I believe it could be the case for the error propagation expressions as well but they are very complex, so maybe not.

• What are the major differences between AST and HIR? From what I've seen so far (and done in this PR), it's mostly a 1-to-1 translation.

It is a 1-to-1 translation except in the cases where it can be simplified, as well as for AST nodes which cannot exist this late in the pipeline (macro invocations for example). There are some exceptions to this rule like the asm! macro whose content needs to be available up until the codegen stage.

• What does this Analysis::NodeMapping thing even do?
  • I'm not dumb, I'm just not very familiar with GCC / gccrs internals, that's all 🙂

The NodeMapping class helps in keeping track of HIR nodes and the AST node they correspond to as well as the crate they originate from. The class is defined in rust-hir-map.h. It's complicated but very useful :D And this isn't a dumb question, it's not an easy thing to grasp nor do we have a lot of documentation about it.

• For desugaring, I need to look up a trait marked with lang = "try"; how would I do that? Is there some sort of lang item -marked item registry where I could do something like registry ()->look_up (RustLangItem::TRY)?
  • I need to look into how traits for arithmetic ops are looked up and just do the same. But maybe you can just tell me the answer straight up.

I found this:

gccrs/gcc/rust/typecheck/rust-hir-type-check-expr.cc

Lines 1581 to 1595 in 74b4d6d

	bool
	TypeCheckExpr::resolve_operator_overload (
	Analysis::RustLangItem::ItemType lang_item_type, HIR::OperatorExprMeta expr,
	TyTy::BaseType *lhs, TyTy::BaseType *rhs)
	{
	// look up lang item for arithmetic type
	std::string associated_item_name
	= Analysis::RustLangItem::ToString (lang_item_type);
	DefId respective_lang_item_id = UNKNOWN_DEFID;
	bool lang_item_defined
	= mappings->lookup_lang_item (lang_item_type, &respective_lang_item_id);
	// probe for the lang-item
	if (!lang_item_defined)
	return false;

yes, that's exactly what you'll have to do. I would suggest also looking at

gccrs/gcc/rust/typecheck/rust-hir-type-check-expr.cc

Lines 1457 to 1579 in 74b4d6d

	void
	TypeCheckExpr::visit (HIR::ClosureExpr &expr)
	{
	TypeCheckContextItem &current_context = context->peek_context ();
	TyTy::FnType *current_context_fndecl = current_context.get_context_type ();
	HirId ref = expr.get_mappings ().get_hirid ();
	DefId id = expr.get_mappings ().get_defid ();
	RustIdent ident{current_context_fndecl->get_ident ().path, expr.get_locus ()};
	// get from parent context
	std::vector<TyTy::SubstitutionParamMapping> subst_refs
	= current_context_fndecl->clone_substs ();
	std::vector<TyTy::TyVar> parameter_types;
	for (auto &p : expr.get_params ())
	{
	if (p.has_type_given ())
	{
	TyTy::BaseType *param_tyty
	= TypeCheckType::Resolve (p.get_type ().get ());
	TyTy::TyVar param_ty (param_tyty->get_ref ());
	parameter_types.push_back (param_ty);
	TypeCheckPattern::Resolve (p.get_pattern ().get (),
	param_ty.get_tyty ());
	}
	else
	{
	TyTy::TyVar param_ty
	= TyTy::TyVar::get_implicit_infer_var (p.get_locus ());
	parameter_types.push_back (param_ty);
	TypeCheckPattern::Resolve (p.get_pattern ().get (),
	param_ty.get_tyty ());
	}
	}
	// we generate an implicit hirid for the closure args
	HirId implicit_args_id = mappings->get_next_hir_id ();
	TyTy::TupleType *closure_args
	= new TyTy::TupleType (implicit_args_id, expr.get_locus (),
	parameter_types);
	context->insert_implicit_type (closure_args);
	Location result_type_locus = expr.has_return_type ()
	? expr.get_return_type ()->get_locus ()
	: expr.get_locus ();
	TyTy::TyVar result_type
	= expr.has_return_type ()
	? TyTy::TyVar (
	TypeCheckType::Resolve (expr.get_return_type ().get ())->get_ref ())
	: TyTy::TyVar::get_implicit_infer_var (expr.get_locus ());
	// resolve the block
	Location closure_expr_locus = expr.get_expr ()->get_locus ();
	TyTy::BaseType *closure_expr_ty
	= TypeCheckExpr::Resolve (expr.get_expr ().get ());
	coercion_site (expr.get_mappings ().get_hirid (),
	TyTy::TyWithLocation (result_type.get_tyty (),
	result_type_locus),
	TyTy::TyWithLocation (closure_expr_ty, closure_expr_locus),
	expr.get_locus ());
	// generate the closure type
	NodeId closure_node_id = expr.get_mappings ().get_nodeid ();
	const std::set<NodeId> &captures = resolver->get_captures (closure_node_id);
	infered = new TyTy::ClosureType (ref, id, ident, closure_args, result_type,
	subst_refs, captures);
	// FIXME
	// all closures automatically inherit the appropriate fn trait. Lets just
	// assume FnOnce for now. I think this is based on the return type of the
	// closure
	Analysis::RustLangItem::ItemType lang_item_type
	= Analysis::RustLangItem::ItemType::FN_ONCE;
	DefId respective_lang_item_id = UNKNOWN_DEFID;
	bool lang_item_defined
	= mappings->lookup_lang_item (lang_item_type, &respective_lang_item_id);
	if (!lang_item_defined)
	{
	// FIXME
	// we need to have a unified way or error'ing when we are missing lang
	// items that is useful
	rust_fatal_error (
	expr.get_locus (), "unable to find lang item: %<%s%>",
	Analysis::RustLangItem::ToString (lang_item_type).c_str ());
	}
	rust_assert (lang_item_defined);
	// these lang items are always traits
	HIR::Item *item = mappings->lookup_defid (respective_lang_item_id);
	rust_assert (item->get_item_kind () == HIR::Item::ItemKind::Trait);
	HIR::Trait *trait_item = static_cast<HIR::Trait *> (item);
	TraitReference *trait = TraitResolver::Resolve (*trait_item);
	rust_assert (!trait->is_error ());
	TyTy::TypeBoundPredicate predicate (*trait, expr.get_locus ());
	// resolve the trait bound where the <(Args)> are the parameter tuple type
	HIR::GenericArgs args = HIR::GenericArgs::create_empty (expr.get_locus ());
	// lets generate an implicit Type so that it resolves to the implict tuple
	// type we have created
	auto crate_num = mappings->get_current_crate ();
	Analysis::NodeMapping mapping (crate_num, expr.get_mappings ().get_nodeid (),
	implicit_args_id, UNKNOWN_LOCAL_DEFID);
	HIR::TupleType *implicit_tuple
	= new HIR::TupleType (mapping,
	{} // we dont need to fill this out because it will
	// auto resolve because the hir id's match
	,
	expr.get_locus ());
	args.get_type_args ().push_back (std::unique_ptr<HIR::Type> (implicit_tuple));
	// apply the arguments
	predicate.apply_generic_arguments (&args);
	// finally inherit the trait bound
	infered->inherit_bounds ({predicate});
	}

which does the same lookup as well as some error handling.

(by the way, instead of just a return false this should probably emit a warning saying it could not find the required lang item?)

But also, the following looks just wrong:

gccrs/gcc/rust/typecheck/rust-hir-type-check-expr.cc

Lines 1597 to 1616 in 74b4d6d

	auto segment = HIR::PathIdentSegment (associated_item_name);
	auto candidates
	= MethodResolver::Probe (lhs, HIR::PathIdentSegment (associated_item_name));
	bool have_implementation_for_lang_item = candidates.size () > 0;
	if (!have_implementation_for_lang_item)
	return false;
	if (candidates.size () > 1)
	{
	// mutliple candidates
	RichLocation r (expr.get_locus ());
	for (auto &c : candidates)
	r.add_range (c.candidate.locus);
	rust_error_at (
	r, "multiple candidates found for possible operator overload");
	return false;
	}

We should not attempt to resolve a method named, say, add, but instead solely look at whether or not the receiver implements the one trait marked with the lang item. And indeed, testing the following:

#[lang = "add"]
pub trait Add<Rhs = Self> {
    type Output;
    fn add(self, rhs: Rhs) -> Self::Output;
}

trait AlsoAdd {
    fn add(self);
}

struct MyStruct(i32);

impl Add for MyStruct {
    type Output = i32;
    fn add(self, rhs: MyStruct) -> i32 {
        self.0 + rhs.0
    }
}

impl AlsoAdd for MyStruct {
    fn add(self) { }
}

fn foo() ->i32 {
    MyStruct(35) + MyStruct(42)
}

I get the error:

bad-add.rs:25:5: error: multiple candidates found for possible operator overload
   15 |     fn add(self, rhs: MyStruct) -> i32 {
      |     ~~
......
   21 |     fn add(self) { }
      |     ~~
......
   25 |     MyStruct(35) + MyStruct(42)
      |     ^~~~~~~~
bad-add.rs:25:5: error: cannot apply this operator to types MyStruct{MyStruct (0:i32)} and MyStruct{MyStruct (0:i32)}
bad-add.rs:25:5: error: failed to type resolve expression

Who cares whether or not there happens to be another method named add on my type! I'm not invoking .add(), I'm using +, the Add trait. rustc playground

and I agree with this. Could you open an issue with the above code? it would be nice to have error handling here as well if the add lang item is not defined.

[bugaevc]

Thank you!

Could you open an issue with the above code?

I'd rather open a pull request :D but for that, please reply to the second set of questions.

[philberty]

I think some of this will be a bit simpler when i merge my work over here: #2140

Traits a complicated, so you can have a plain basic trait, so we need a way to work with them which is what the TraitReference object is, which is mostly just a helper object around it. The other part is the TypeBoundPredicate Rustc has something similar cant remember the name but this is all about managing generic traits.

Generic Traits are a pain and the current master code has one main issue here, where Trait has an implicit Self generic type parameter. This is left as a generic in the case of the Self type bound predicate but in every other case such as:

impl Foo for u8 {...}

This means we really create a type-bound predicate to wrap up the generics here properly which means we end up with

Foo<u8, i32>

The current master code does not track the Self parameter properly here which is confusing but i have fixes on the way. This is all required to handle complicated cases of associated types and trait disambiguation.

The other part to be aware of the is another bug in master for qualified path syntax such is always resolving the to the trait item and we are relying on code generation to always monomorphize it which is wrong again i have fixes for this laready.

You test case:

struct SomeItem<T>;

trait Foo {
    type Bar;
}

fn my_func<U: Foo>() {
    SomeItem<U::Bar>;
}

I havented chaged if this is working or broken but here this is a generic unit struct the generic U has a type bound predicate of Foo and then we its argument is the trait item of Bar but i have a feeling if this is not working its likely missing the piece i mentioned earlier thats not tracking the Self type properly here.

Associated types like this are really awkward because assoicated types can be the one implemented on the impl block or the placeholder on the trait which will need setup if possible when we get more information. So in the case the best case you can hope for us that SomeItem will turn into SomeItem because there is not enough type info to get any further.