Stream: t-compiler

Topic: generic argument disambiguation


jplatte (Oct 08 2019 at 22:02, on Zulip):

Hi, I've started work on disambiguation of identifiers as generic arguments between regular type generics and const generics. @varkor you wrote a comment here where you outlined a rough approach to this. I've now completed most of the work in libsyntax, but in HIR lowering I don't know what addition information I might have there that allows disambiguating at that stage. Could you give me some pointers?

jplatte (Oct 08 2019 at 22:03, on Zulip):

What I've done so far is at up at https://github.com/jplatte/rust/tree/generic-arg-disambiguation, in case anyone wants to look at it.

Vadim Petrochenkov (Oct 09 2019 at 07:31, on Zulip):

@jplatte
AST changes are unnecessary, see the later comment - https://github.com/rust-lang/rust/issues/60804#issuecomment-516769465

Vadim Petrochenkov (Oct 09 2019 at 07:33, on Zulip):

Everything can be done in resolve (visit_generic_arg) and AST -> HIR lowering (lower_generic_arg).

jplatte (Oct 10 2019 at 16:53, on Zulip):

Does it make sense to do it that way though? The type variant can represent a bunch of stuff beyond just identifiers that aren't allowed for const generics. Filtering those out from the value resolution fallback seems more error-prone than distinguishing those cases when parsing.

Vadim Petrochenkov (Oct 10 2019 at 17:45, on Zulip):

AST preserves all the data necessary to discern between an ident and an arbitrary path, it's segments.len() == 1 && segments[0].arg.is_none(), not too error prone.
The alternative is to make two variants that are supposed to behave identically almost everywhere - keeping them synchronized does seem more error prone (and taking more code).

Vadim Petrochenkov (Oct 10 2019 at 17:46, on Zulip):

Pat::Ident is separate from Path::Path because it also covers things like ident @ PAT or mut ident, otherwise it would also make sense to merge them.

centril (Oct 10 2019 at 17:49, on Zulip):

it's segments.len() == 1 && segments[0].arg.is_none(), not too error prone.

@Vadim Petrochenkov maybe abstract this tho? probably used in other places

Vadim Petrochenkov (Oct 10 2019 at 17:54, on Zulip):

If it's used multiple times, then sure, why not.

jplatte (Oct 10 2019 at 19:29, on Zulip):

But segments.len() == 1 && segments[0].arg.is_none() would be pretty restrictive, no? Why should e.g. (associated) paths to constants not be disambiguated too?

Vadim Petrochenkov (Oct 10 2019 at 19:56, on Zulip):

We need to disambiguate in favor of types (for backward compatibility), and for multi-segment paths type-based resolutions are unavailable if we are disambiguating before/during lowering to HIR.
So it may happen that the path resolves to a value but not a type during early resolution, but resolves to a type during type-based resolution.

Vadim Petrochenkov (Oct 10 2019 at 19:58, on Zulip):

If the disambiguation is delayed until the last moment (generic argument substitution), then we can disambiguate multi-segment paths.

Vadim Petrochenkov (Oct 10 2019 at 20:00, on Zulip):

Hm, the arg.is_none() condition should be unnecessary though.

jplatte (Oct 10 2019 at 23:09, on Zulip):

Okay, so that means disambiguation has to (partially?) happen later, right? But I don't need to introduce new GenericArg variants for that? Just some changes to resolve and lowering in libsyntax and some other things later? I can try making that work :)

yodal (Oct 20 2019 at 18:44, on Zulip):

@Vadim Petrochenkov I am trying to pick this up from @jplatte and I just want to make sure I am understanding what you were suggesting. If I am reading everything correctly, you are suggesting passing a path through ast::GenericArgs that could either be a type or a constexpr. Once we hit AST -> HIR we check for segments.len() == 1 && segments[0].arg.is_none() to check if it is an ident, and from there check if it is a type or const argument, in that order. Is that all correct or am I completely off in the weeds?

varkor (Oct 20 2019 at 23:13, on Zulip):

I think the idea is that a const parameter would initially be a GenericArg::Type with a single segment and no arguments — when we try to resolve the path in the type namespace, if it doesn't exist, then we also try in the value namespace — and if that succeeds, we lower to a const argument instead

varkor (Oct 20 2019 at 23:14, on Zulip):

which I think is what you're describing

yodal (Oct 21 2019 at 01:11, on Zulip):

ok, that's more or less what I was thinking, though coming at it from a slightly different direction

Last update: Nov 22 2019 at 04:55UTC