If the argument type did not match positionally and needed to be converted, it would need to be placed after the ellipses, using roughly the existing Get syntax. i.e. this example from the description:

await the_rule_to_call(..., Arg1())

I think for clarity that should be Arg3 then? And state somewhere there's a rule for Arg3 -> Arg1?

#18905 (comment)

I imagine the same thing would hold as today for this case, i.e. the engine would need to go out and > find the chain of rules to use to get the required type from what you provided.

Those examples are mostly accurate, except that your Gets would need the return type, which in the proposal is inferred from the return type of the named rule:

# Anything after ellipsis is equiv. with regular `Get`, but the return type is not mentioned:
await the_rule_to_call(..., *args)
await Get(ReturnType, *args)

Possibly... usually @union callsites also mention the expected return type (because the @union is an input type), which you wouldn't have in this example:

await Car(Volvo(...))

Correct.

Oh wow. Yeah OK, uhhh let's figure this thing out 😄

Yeah type-checkers will be the issue here. Nothing us clever folks can't solve though 😉

I believe that it is possible for it all to be type safe via proper typing of the @rule decorator: https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators

...certainly the trailing ellipses and Get syntax can be. Whether the optional positional args can be will be trickier.

But honestly, if positional args are not supported for by-name @rule invokes, that probably wouldn't be too much of a hardship.

In hindisght, this should've been obvious...

Maybe we can extend the interface so that we can call rules directly AND use Get.

Whenever calling rules within the same backend, the rules should just call each other.
If using core rules, or rules from other backends, Get is a nicer interface so that you can continue treating the rules as a black box.

Maybe we can extend the interface so that we can call rules directly AND use Get.

Whenever calling rules within the same backend, the rules should just call each other. If using core rules, or rules from other backends, Get is a nicer interface so that you can continue treating the rules as a black box.

Get comes at a high cost though: that's the key reason for this proposal. Rule graph solving has become untenably slow/complicated. We are additionally paying the cost of constructing/defining disambiguating wrapper types: many @rules would be completely fine directly returning, say, FrozenDict, rather than wrapping in other types.

But to your point: the only difference in opaqueness in the proposal is that you go from needing to memorize a return type name (await Get(ReturnType, ...)) to needing to memorize/learn the implementing @rule name (await rule_name(...)). So I would argue that it is similarly opaque.

Looping back on this: for now, I have not changed the name of MultiGet, but #20011 added support for using MultiGet with by name calls. We should probably still rename it (possibly even as part of #20572), but I haven't gotten to it.

We could get more clever.

Something like:

• make every argument keyword only (and change the engine for it)
• the request argument uses a new generic: ConvertibleTo[RequestType] the generic is just RequestType (so it can be type-checked as such) but it's a hint to callers that can provide either a RequestType or something convertible to it

To revise my answer slightly: more than just "familiarity", I would say that symmetry with how you call "rule helpers" (i.e., plain async functions which happen to yield Gets currently) is good.

If what we are able to do is reduce the magic involved in calling a @rule to "the remaining arguments are filled in automatically, and the result is memoized", then it will be much easier to toggle between @rules and using plain async calls... which we should likely be using a lot more of to reduce memoization and object creation overheads.

So I'm not sure I understand the distinction between await the_rule_to_call(Arg1(), ...) and await the_rule_to_call(..., Arg1()) ...

If we chose to allow positional args, as originally proposed at the top of this discussion, the behavior would be as described at the top of this discussion: await the_rule_to_call(Arg1(), ...) and await the_rule_to_call(..., Arg1()) would be equivalent, because Arg1 is the first argument of the rule being called. The idea would be that all of the arguments before the ellipses must be valid positional args to the @rule, while the syntax after the ellipses would effectively be approximately "type-driven and possibly-converted kwargs" (the same as the current Get syntax).

I'm not sure that we can make pre-ellipses positional args mypy checkable though, so it might not actually be worth doing: unclear.

Yeah, I wonder if the pre-ellipsis is worth it, in terms of extra confusion. And in that case do we in fact need the ellipsis at all?

didn't think this was an issue no more. In different files it's no issue (right?) and in the same file it will now throw an error at load time.

The traces come back misleading

I think you can get away without the ellipsis if we did the keyword thing, no?

Or even without the keyword thing if we don't bother with positional args and always do:

await the_rule_to_call(Arg_x(), Arg_y())

Where any missing args are filled in, and so the order of Arg_x() and Arg_y() don't actually matter?

FWIW, this can be made type-safe with positional args if the magic implicit() keyword/function doesn't have a return type:

def implicit():
    pass

def my_rule(arg1: str, arg2: int) -> str:
    return "yep"

my_rule(**implicit())
my_rule("one", **implicit())
my_rule("one", 2, **implicit())
my_rule(2, "one", **implicit())

example.py:29:17: error: Argument 1 to "my_rule" has incompatible type "int"; expected "str"  [arg-type]
            my_rule(2, "one", **implicit())
                    ^
example.py:29:20: error: Argument 2 to "my_rule" has incompatible type "str"; expected "int"  [arg-type]
            my_rule(2, "one", **implicit())
                       ^
Found 2 errors in 1 file (checked 3 source files)

Or even without the keyword thing if we don't bother with positional args and always do:

No positional args means no useful type checking, which is a bummer I think.

I think this might be a better path forward.

I want to make sure the typing is completely sound and editors can join in.

I also want to see if we can eliminate request types this way as well

Although I think every example so far is unions...

This makes me think of one minor downside to this, and that is that naming things is close to the top on the list of the most difficult things in computer science. So exposing more names in the API will be a cause for headaches at some point. Not saying this is reason to not do this, just worth considering I think.

Where does the request= name come from in this example?

await the_rule_to_call(request=Arg1())

What would it look like to pass an argument which was going to be converted from the actual passed argument type? That still wouldn't be type safe, I imagine...

From the definition of the_rule_to_call. And yes all type safety is/was already off the table because of conversion semantics.

Unfortunately I don't think there's a way in the type system to model this at the call-site. Ideally we keep the names and map the types to Any, but...

Good point. Rule collection might look different as well. That's probably a good thing?

I kind of liked the disconnect between a type and the rule implementation.

I've thought about that some. I like this way better, and I'd argue it's "more correct".

• Human correctness: The code reader is able to reason about the flow of code better, and more importantly tools can assist (e.g. "jump to definition" in your IDE)
• Graph Correctness: Currently if the request type and rule are in two different files, --changed values, including transitive, don't do the right thing if you edit a rule. Even with this the graph will still be wrong, but is a closer approximation to the truth

I don't get the correctness arguments.

• Human correctness: The code reader is able to reason about the flow of code better, and more importantly tools can assist (e.g. "jump to definition" in your IDE)

I don't want to reason about the flow of code (but about the flow of data, which is about the types involved) I want to focus on an isolated part of a bigger question trusting that the engine will provide the rest.

Either way, I don't see these contrasts as more or less correct than the other.

Graph Correctness: Currently if the request type and rule are in two different files, --changed values, including transitive, don't do the right thing if you edit a rule. Even with this the graph will still be wrong, but is a closer approximation to the truth

This would be during Pants/plugin development only, I guess? What do you mean with that the graph will be wrong?
If I edit a rule, and I want to run tests for all changed rules, tests that depend on that rule will be included with --changed, right?

#20011 made rule helper and by-name calls identical, including support in MultiGet.

await ConcurrentRules()?

asyncio calls this gather: https://docs.python.org/3/library/asyncio-task.html#asyncio.gather

Not sure it makes much sense here though.

All or Concurrently wrapper classes would be easy to read.

Yea, I like All since it's short (and because we had also discussed introducing a FirstOf type for speculation).

#20011 added support for using the call-by-name syntax with MultiGet, but did not yet change the name: choosing a new name will be a blocker for any large-scale rewrite to the new syntax though.

**context(...) ?

It is somewhat descriptive

TBD! It might still be type driven.

@benjyw @stuhood Where did we land here?

I'm working on migrating files, but I'm currently staying as far "out" as possible in the backends - where everything is already pretty concrete.

Still TBD. I would like a non-type-driven solution if possible. I'll think about it.

I think you're right. And things like this is why I'm not 100% happy with the call-by-name move as I rather liked the loose coupling we get with the typed rule system. But I realize it all comes with trade offs, and I'm not in a position where I can invest a lot of time to figure out a better approach here.. ;)

I'm ok with call-by-name thing, but it would be nice to have something like unittest.mock.patch but for rules