| |
| Posted by Andrei Alexandrescu in reply to Walter Bright | PermalinkReply |
|
Andrei Alexandrescu
Posted in reply to Walter Bright
| On 5/1/21 4:08 AM, Walter Bright wrote:
> On 4/30/2021 3:03 PM, Andrei Alexandrescu wrote:
>> ```D
>> template staticIndexOf(T, TList...)
>> {
>> enum staticIndexOf = genericIndexOf!(T, TList);
>> }
>>
>> template staticIndexOf(alias T, TList...)
>> {
>> enum staticIndexOf = genericIndexOf!(T, TList);
>> }
>> ```
>
> I'm curious why genericIndexOf is not renamed to staticIndexOf and the previous staticIndexOf templates removed.
I don't know. I speculate that the following sequence of events happened:
1. Someone writes `staticIndexOf` "reasonably" with types (could have been aliases, but I recall the first historical uses were with types):
/** ... docs ... */
template staticIndexOf(T, TList...) { ... }
2. Someone else figures staticIndexOf should also work with values. They try to add "alias" to `T` in there but unittests fail due to bugs and undue limitations in the compiler. They figure the right solution is to add an "overload":
/** ... docs ... */
template staticIndexOf(T, TList...) { ... }
/// ditto
template staticIndexOf(alias T, TList...) { ... }
This does not upset the names in the documentation (they are still `T` and `TList`) so it's an entirely transparent and reasonable way to work around said bugs and undue limitations. That contributor might have thought that there may be subtle reasons for which `alias` parameters don't accept type arguments transparently, so they perhaps chose not to protest it all that much. The reviewer of the code might have accepted the same reasoning with a sigh.
3. Someone else (or the same contributor in the same PR) figures there's a way to eliminate the duplication in code:
/** ... docs ... */
template staticIndexOf(T, TList...)
{
enum staticIndexOf = genericIndexOf!(T, TList);
}
/// ditto
template staticIndexOf(alias T, TList...)
{
enum staticIndexOf = genericIndexOf!(T, TList);
}
// private
private template genericIndexOf(args...) { ... }
This does not upset the documentation and remains transparent to the user, so it seems like a reasonable thing to do.
4. Other people (or, again, the same contributor in that or other PRs) figured the same pattern can be applied to other artifacts as well. And reviews accept that code because otherwise it's impossible to make the unittests pass.
5. Now the pattern is an accepted way of doing things whenever an artifact must work with both types and aliases. The concern that the pattern scales poorly and indicates a massive problem with the language was not raised; after all, we had working code using the pattern.
This has happened before, and will happen again unless we do something about it.
>> This code should have NEVER been accepted upon review. Instead, the reviewers should have filed a TOP PRIORITY bug report to dmd "Not binding types to alias templates forces unscalable code duplication for all typelist primitives".
>>
>> This needs to be fixed, and very urgently.
>
> Please file a bug report!
Apparently Nick Treleaven proposed a solution or a part of it:
https://github.com/dlang/dmd/pull/11320
The bug report related to the process is more subtle and does not belong in bugzilla.
|