I think this should go in the call implementation in SimpleValue. In general, we should consider dynamic_casting an anti-pattern in this part of the code.

That doesn't work, because you have to add the classtype value as the self argument. In the test example:
MyClass(1) inputs here are just 1. You need to check here if we're calling a classtype so that we can add the callee to the list of arguments, otherwise at the point of SimpleValue::call the callee will be lost.

Not sure I understand—why can't you do that in SimpleValue::call()? You have access to self there.

Ahh got it to work. I tried the way you suggested previously but I forgot to emplace the self argument.

You are adding self to inputs here, only to slice it off in callClassMethod above. We seem to be piling pieces on top of each other here; it's not clear that callClassMethod should exist at all—could not be expressed as a composition of operations on sugared values? attr lookup (to get the __call__ MethodValue), then calling the method with the provided arguments.

an attr lookup above would give the error message:

Tried to access nonexistent attribute __call__. Did you forget to initialize it in __init__()?:

The error message with callClassMethod is:

Foo does not define a __call__ method

which is a better error message. separately, looking up an attr and trying to call it is not actually correct - if a class has an attribute __call__ that is callable but is not a method than you can not invoke the class as a callable.

Also - asking to inline a function that expresses a common pattern (it's invoked three times in this file) seems like a nit to me. Could we accept PRs when the only remaining comments are nits?

This code does seem like a bandaid. Attribute lookup is how we always lookup methods. If the error message is bad here it is also bad for standard method lookup. It's fine to want to improve that but it is not ok to just fix it in this instance and add more code to maintain. I do not think it is a good habit to think these kinds of things are nits. By adding more pathways here rather than using existing ones, it leaves the code in worse shape than where we started.

The error message is bad because attr lookup in general does not have the additional context that we are looking for a method instead of another type of attribute (did you forget to initialize it in __init__()?. There isn't another pathway besides magic methods in which you know the attr is expected to be a method. In other cases our error messages have been overly general and not applied to the actual problem at hand and it's led to confused users (did you forget to add it to __constants__?).

This is specializing an error message when we know more information. That can't be a general anti-pattern - so in this case are the comments just saying that the additional specialization isn't worth the improvement in error message ?

Attribute lookup is how we always lookup methods. This means the error for missing methods is always this bad. This code fixes it for the case of only call __call__. I am asking that is is either fixed for all methods or not fixed at all. The middle ground is more code to maintain and is still not good.

I removed callClassMethod. It fixed it for all magic methods not just __call__ (which is the only time you know you're looking up a method) but anyway I updated the PR to remove the extra path.