Issue 23473: Improvement: Implement single-argument __traits(getOverloads) form.

[FeepingCreature]

https://issues.dlang.org/show_bug.cgi?id=23473

Rationale:

At the moment, when looking at an overload set passed via an alias parameter to a template, there is no way of extracting the individual functions making up the overload set. In the ordinary case, you can use a hack like __traits(getOverloads, __traits(parent, sym), __traits(identifier, sym)), but this fails as soon as the overload set contains alias renames or other modules. But __traits(getOverloads) internally looks at a symbol anyways, so we just redefine the single-argument form (currently an error) to pass that symbol directly.

Should I make a spec PR first?

[dlang-bot]

Thanks for your pull request and interest in making D better, @FeepingCreature! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✗	23473	enhancement	Need a way to disassemble an overload function without referencing a parent

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14631"

[RazvanN7]

The behavior in the presence of imported overload sets needs to be defined:

// a.d
void foo(int);
void foo(string);

// b.d
void foo();

// c.d
import a;
import b;

__traits(getOverloads, foo);   // which one?!

[RazvanN7]

Also, please add "Fix" in the commit message so that the bot auto-closes the issue if this gets merged.

[RazvanN7]

@FeepingCreature I've tested a bit locally and this seems to be a problem only if the overload set is defined and used in the same module. For example, this works:

// b.d
void first(int) { }

// c.d
void second(string) { }

// a.d
import b : foo = first;
import c : foo = second;

// d.d

import a;

void main()
{
    alias overloads = __traits(getOverloads, a, "foo");
    static foreach(ov; overloads)      
        pragma(msg, __traits(identifier, ov));
}

So if you know where the overload set is defined, things work correctly.

In this light, I would argue that we do not need this addition. getOverloads needs to be pointed out where to pick up the overload set because that offers maximum flexibility. Instead of adding more complexity to the language we can simply pinpoint the file where the overload is constituted.

[FeepingCreature]

So a template that takes an alias parameter now needs to be passed the calling module and a string??

I think you're falling into C++ thinking a bit here. You shouldn't ask "can we do this with what we already have", but "is this good or is this bad?" I think doing it that way is bad. Nobody looks at "Yeah you can't write templ!foo, you need to write templ!(net.foo.CurrentModule, "foo")" and thinks "Yeah, this is a good, well-designed language I should recommend to others."

[RazvanN7]

I agree, it's just that I wonder if this form of getOverloads is not susceptible of introducing weird edge cases.

I guess the question is: how often do we find ourselves in the situation presented in the bug report. If it happens a lot, then I guess a case could be made for this addition. If it's not that often, then maybe we should just stick with what we have and accept that you need to jump through some extra loops in some rare cases.

cc @WalterBright @atilaneves

[FeepingCreature]

To give some extra context, my usecase is that I want to pass an overload set to the alias parameter of a function and apply it to sumtype.match "as if it were a bunch of lambdas I've passed individually". Basically, the goal is to use match's lambda selection to implement a SFINAE-in-userspace.

In effect, this will look like foo.subscribe!(getKey, T) ... alias getKey = a => a.key; alias getKey = a => a.value.key; etc, each overload listing one possible way of extracting a key from a subtype of T. Very elegant! However, to do that I need to resolve getKey back into its constituent alias declarations.

This is how the code looks right now:

auto getKey(T)(T value)
if (__traits(hasMember, T, "key"))
{
    return value.key;
}

And repeat. Workable, but elegant by no means whatsoever.

That said, my argument is less "I can't do that unless I have this feature." I don't need need this feature. My argument is this: an overload just is a symbol. Why does __traits(getOverloads) not operate on them? I don't get why the construction of this trait takes on this extra verbosity and particularity.

[atilaneves]

Overload set rules are complicated, and because of that and the fact that I didn't come up with the ones for D, I'll have to leave this one to @WalterBright

[pbackus]

Basically, the goal is to use match's lambda selection to implement a SFINAE-in-userspace.

Slightly off-topic: I'm not sure that replacing D's built-in overload selection with match's handler selection is actually a great idea here. Unlike D's native overload selection, match uses the order of the handlers to resolve ambiguous cases. Using it like this would make your code sensitive to the order in which __traits(getOverloads) lists the individual overloads, which in practice is declaration order but officially is unspecified and could vary between compiler releases.

[FeepingCreature]

Offtopic, but I mean, that's something that can be fixed in code - if there's a way to access the overload members.

[FeepingCreature]

I just ran into an issue where I'd be able to make really good use of this, again. What's blocking it, what can I do?

[thewilsonator]

A rebase and a response from @WalterBright by the looks of it, failing the latter, just the former.

I would have expected a larger test case, but I suppose the feature is narrow enough that that is all that is required to cover it.

[FeepingCreature]

If you have any ideas for more tests, I'm all ears. It really is fairly simple.

Rebased. It's a bit annoying of a diff because it adds an indentation.

I would like some feedback from Walter because I don't think the stylistic approach is very good.

[RazvanN7]

@FeepingCreature Please add this test case:

// a.d
void foo(int);
void foo(string);

// b.d
void foo();

// c.d
import a;
import b;

__traits(getOverloads, foo);   // which one?!

And also clarify the behavior.

[FeepingCreature]

Thanks a bunch! That testcase actually surfaced an issue; which to fix I had to adjust the behavior of D overload resolution in general, so now I really want Walter to review before merging.

Shouldn't have a user-visible impact, but let's see what the testsuite says.

[RazvanN7]

cc @WalterBright @atilaneves as these is a new language addition.

[FeepingCreature]

ping

(Sure would have been good to have this now that the __traits(getAttributes) deprecation is forcing lots of __traits(getOverloads) usage...)

[FeepingCreature]

It sounds like this is not retrieving overload sets at all, it is retrieving all the members of all the overload sets with the given name. Is this correct?

The thing I'm aiming for is: "Given a symbol (usually resolved from an identifier), retrieve all the concretely-callable things (functions, aliases, structs with opCall), ie. the overload leaves, that would be considered if you tried to call this symbol at this location."

I thought this was what getOverloads was already doing.

[WalterBright]

I thought this was what getOverloads was already doing.

It was not recursively expanding overload sets. For example, if this were pulled as it is, then getOverloads would provide a tuple of overloads with the overload sets information removed, meaning it could not be used as a substitute for overload resolution. The anti-hijacking information will be lost along with the overload sets.

This is why I question the utility of the proposed feature. It will lump together functions that were never designed to be overloaded with each other, they just happened to have the same name.

[FeepingCreature]

But if they're referenced by name in the current module, aren't they by definition overloaded with each other (just by virtue of having the same name)? That's how I've been using them. I don't understand when you'd want to use getOverloads to split one symbol up into groupings on the basis of modules, when this is not something you could ever do via a function call. Like, I don't understand the usecase at all here.

Example, paraphrased from actual production code:

import foo.Decoding : decode;
import bar.Decoding : decode;

void setup() {
    ...
    // `decode` is one symbol combining the overload set from `foo.Decode` 
    // and `bar.Decode`, with origin indistinguishable.
    // When would you want RestService to tease them apart by module, but not by individual function?
    // The whole goal here is to glom them together into one undifferentiated ball.
    auto service = new RestService!(IService, encode, decode)(new ClientImpl(config.address));
    ...
}

[WalterBright]

But if they're referenced by name in the current module, aren't they by definition overloaded with each other (just by virtue of having the same name)?

See: https://dlang.org/articles/hijack.html

[FeepingCreature]

Ah, I see. I wasn't aware of the additional wrinkle that exactly one set had to match. That makes the design make a lot more sense!

So should/could getOverloads do something like...

module A;
void foo(long);

module B;
void foo(int);
void foo(float);

module C;
import A, B;
static assert(is(Parameters!(__traits(getOverloads, foo)[0]) == long));
static assert(is(Parameters!(__traits(getOverloads, __traits(getOverloads, foo)[0])[0]) == int));

As long as I can eventually get at all the leaves, it should do for my purposes.

[FeepingCreature]

Removed the change in func.d. Instead, recognize when we're seeing an overloaded function in __traits(getOverloads) and recurse into it.

This is a bit iffy: it only affects implicit overloads created by importing the same name multiple times. Explicit overloads, created with alias name = foo.name; alias name = bar.name;, will already recurse into every member. Because I have no good way to detect when overloadApply decided to do this, I just remove double visits to the same function. Theoretically, the alias I add to the result tuple should be an overload, so I can do the double __traits(getOverloads) I mentioned in the comment, but it isn't and I don't understand why.

(Though I'm honestly fine with this hack, because I don't think anyone else understands alias/overload management either.)

[FeepingCreature]

ping

[WalterBright]

module A;
void foo(long);

module B;
void foo(int);
void foo(float);

module C;
import A, B;

I don't understand what __traits(getOverloads, foo) should do with this that would be useful. If it recurses into the overload sets, information about which overload sets they go into will be lost, and hence there will be no way to reconstruct it. You'll have foo(long), foo(int), foo(float), and what use will that be? module A was not designed with any consideration for foos in module B, and vice versa. The foos may be for utterly different purposes.

[FeepingCreature]

Spoken frankly: it seems to me you are giving a lot of priority to this way of avoiding overload collisions, whereas from my point of view the originating module of a symbol is basically never relevant. I don't think this has ever mattered to me, and honestly I'm not sure it's ever mattered to anyone else - observe Buildkite remaining green after the overloadApplyRecurse change that broke the whole thing. I want this because the one case I care about resolving overloads is when I'm trying to inspect an overload tuple passed to a template alias parameter, and all the functions I'd pass this way are either disjunctive or not resolved by overloading anyways.

For background, the motivation is about serialized, where I'm using overloads to pull together encode/decode helpers from a bunch of different modules (from git submodules) and stuff them into a single encodeJson/decodeJson call. Now everything is good for encoding, because encode helpers are necessarily narrowing; I can just declare them of WireType encode(InternalType) and get a nice healthy overload that I can just try to call with my internal type. But decode helpers are widening; they'd have to be, for instance, InternalType1 decode(JSONValue); InternalType2 decode(JSONValue);. So I can't use overloading here anyways; I couldn't even declare them, because they'd collide on declaration, not on call. So right now I do InternalType1 decode(T : InternalType1)(JSONValue), etc., and I have to do __traits(compiles, decode!T) to check if I have a matcher. However, see my serialized dconf talk, "__traits(compiles) is Satan" - the __traits(compiles) also fails if there's a compile error inside the decode, which can be hard to notice.

So when I saw __traits(getOverloads) it occurred to me there was a better way to do it:

InternalType1 decode()(JSONValue value) {}
InternalType2 decode()(JSONValue value) {}

and so on. Overloads cleanly, and I can just use __traits(getOverloads) to tease them apart, instantiate each with !(), look at the return type with ReturnType and bypass the inadequate-for-my-purposes overload resolution entirely. Except, no, because see above. Hence this PR.

I don't want to emphasize my usecase too heavily. This is meant more as an illustrative example for why someone could want this. The reason I actually went ahead and did the PR is: if you see a trait called getOverloads, you - atleast, I - assume that this is possible. It's a symbol, it's the most natural thing for a function to be. You have a thing, and you just introspect it with the introspection API, __traits(getOverloads, thing). The thing the trait actually does, rather, takes a lengthy explanation and the specific behavior regarding module sources that justifies it doesn't seem to even be used in practice (where buildkite can see it). Least surprise this ain't.

If you want overload behavior, you just call it! Why would you be taking the overload set apart at all if you weren't interested in the properties of its leaves?

[tgehr]

Buildkite is obviously not going to fail by you removing a way that code could fail to type check. This does not show what you say it shows. In any case, is it not confusing to have a __trait(getOverloads, ...) that returns overload sets rather than overloads?

[FeepingCreature]

Ah yeah, good point, sorry.

As per the test, __traits(getOverloads) returns every individual leaf right now.