simplify implementation of isDynamicArray()

[WalterBright]

Rationale

Feature request or issue tracking

Closes #10974.

Pre-review checklist

• I have performed a self-review of my code.
• If my PR fixes a bug or introduces a new feature, I have added thorough tests.
• If my changes are non-trivial and do not concern a reported issue, I have added a changelog entry.

LLM/AI disclosure

This PR was written with the assistance of an LLM/AI.
This PR was written entirely by an LLM/AI.

[pbackus]

Even with the compile error fixed, this is not a valid refactor because it changes existing behavior. Example:

import std.traits;

struct S
{
    int[] a;
    alias this = a;
}

// current behavior - prints "false"
pragma(msg, isDynamicArray!S);

// new behavior - prints "true"
static if (is(S U : U[])))
    pragma(msg, "true");
else
    pragma(msg, "false");

[0xEAB]

Closes #10974.

I don’t think so.

[WalterBright]

#0xEAB I don't think so, either. Somehow I pushed into the wrong branch.

[WalterBright]

@pbackus This is why I detest Phobos.

isDynamicArray has utterly misleading documentation. There are 4 kinds of dynamic arrays:

1. T[ ]
2. enum E : T;
3. enum F : E; // and recursively G:F, etc.
4. struct S { int[ ] a; alias this = a; }

and there's no clue in the documentation which of these is supported. And what it does support is completely arbitrary - one has to carefully read the implementation in both Phobos and the compiler source code.

The correct way to do "traits" is to define them for basic building blocks, and then the user can & | and ^ to get what he needs.

@LightBender

[jmdavis]

Yes, isDynamicArray is a mess. What it would ideally do is only pass for types which are exactly dynamic arrays (so T[]) - no implicit conversions, and no enums. Allowing implicit conversions with traits like this is bug-prone, because an enum does not compile with the same code that its base type compiles with, and types in general which implicitly convert to a particular type do not compile with the same code as that type. The implicit conversion has to be forced to get the same behavior, and templated code doesn't do those sorts of conversions at the call site, which is the only way to safely do the implicit conversion in the general case. To avoid those issues, the traits should be testing for the exact types only.

And that's what I've been doing with the traits in Phobos v3. Anyone who wants to test for implicit conversions can then do explicitly by &&ing that test with a trait like isDynamicArray.

But we cannot change the behavior in v2, because who knows how much code relies on the existing behavior, and changing stuff that ends up in conditional compilation such as template constraints and static ifs can easily result in silent behavioral changes rather than just errors (not that we want to make changes which cause errors in existing code either).

For the most part, we cannot fix anything like this in std.traits, because doing so breaks code. We can potentially change an implementation here and there as long as it results in exactly the same behavior, but we cannot change the existing behavior even if it's bad behavior.

[jmdavis]

@WalterBright If you want to see how this particular trait was improved in Phobos v3, you can see the current version of it here: https://github.com/dlang/phobos/blob/master/phobos/sys/traits.d#L255

The implementation itself is dead simple, the documentation attempts to be very clear and thorough (though it's pretty long given how much it explains), and it has quite a few examples in the ddoc-ed unittest block so that it's clear how the trait works (including additional comments on some of the examples).

This is in stark contrast to v2, which has Detect whether type T is a dynamic array. for its entire documentation, and only a handful of examples.

I am by no means claiming that the current v3 stuff is perfect, but I have gone to great lengths to both try to fix the implementation mistakes in v2 and make the documentation as clear as I can make it.