dmd
dmd copied to clipboard
dlang
remove typesafe variadic functions
Don't be too alarmed yet, I'm planning on removing this feature as it is now redundant with other features. This PR is to find out if anyone at all is using it.
https://dlang.org/spec/function.html#typesafe_variadic_functions
Thanks for your pull request, @WalterBright!
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
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#11124"
See: https://github.com/dlang/druntime/pull/3101 https://github.com/dlang/druntime/pull/3100 https://github.com/dlang/druntime/pull/3099 https://github.com/dlang/druntime/pull/3098
Why do you want to remove it ? Is it blocking something ? In another PR, you mentioned:
The replacement for
T[]...isscope T[].
But it's not quite the same though. There are quite a few place they can be used to provide a nicer API, e.g. here.
Remove it because:
- it is rarely used
- being rarely used, it is underdocumented, overlooked, and buggy
- being rarely used, it is not worth it
- for places where it is used, [ ] surrounding the array elements works
- the efficiency gains from it (stack allocation of the array) has been recently duplicated for using array parameters
- I haven't found a case yet of the class construction feature of it
- While Manu's proposed
...syntax addition does not actually conflict with this, it looks like it conflicts, and will be a point of confusion for users
In order to merit first class language support, a feature needs to be commonly used and have a large advantage. This does not exist for this feature.
Why do you want to remove it ? Is it blocking something ? In another PR, you mentioned:
The replacement for
T[]...isscope T[].But it's not quite the same though. There are quite a few place they can be used to provide a nicer API, e.g. here.
Could you instead use something like this?
Remove it because:
- it is rarely used
(I have personally used it on numerous occasions, but I'll try to pretend I haven't).
- being rarely used, it is underdocumented, overlooked, and buggy
Which bugs are you referring to? I can't say that I've encountered any, though haven't tried it any exotic types such as lazy, or structs with a dtor/cpctor (if there are, the fix would be more trivial than the removal).
- being rarely used, it is not worth it
- for places where it is used, [ ] surrounding the array elements works
I'd say that unless there is an alternative that can be used without breaking anyone's API, then the removal can be worth considering.
Surrounding parameters with [ ] is not such an ideal replacement. Perhaps (T...)(string named, T args) with a template constraint to check that all types in T are convertible to a common type is though.
Could you instead use something like this?
Yes, that could help with getting rid of TVF.
Which bugs are you referring to?
I did notice it wasn't handling destructors properly. The lack of use tends to hide these things. There's also figuring out how it should work with @live, scope, attribute inference, etc., the list goes on and on. Each one of those is time for the developers (which is hard to come by) and more and more documentation for the users to plow through.
Benefit / cost is always an issue. Nothing comes for free.
Might be worth pinging @JohanEngelen, as I understand that weka's codebase is template heavy, and TVFs are a kind of template.
There's also figuring out how it should work with @live, scope, attribute inference, etc., the list goes on and on.
Well this wouldn't be the first time a new feature was found to conflict with existing constructs after they got added. For instance, at last check, lazy parameters are still not interoperable with @safe code - there have been bug reports.
I'd say that given we already have a complex matrix of type and storage attributes that it's only natural for new additions to require more and more effort to get established. Removing old features that don't agree with new or introducing ones should not be the first port of calling if we hit upon stumbling blocks though.
I'm thinking of octals as a good example where there were only merits for its removal all round. Do we check all the same boxes here too? I fear not, unless a proposed alternative is compelling enough.
This is indeed used at Weka in about ~50 functions. What I see mentioned here as alternatives are:
foo(T...)(T args)--> no breakage at call site, but separate template instantiation for different number of arguments. Extra code needed to make sure that T[0] == T[1] == T[2].... Extra copying needed when passing the args on to another function that needs an array to work on?foo(T...)(T args) { someotherfunc([args]); }?foo(scope T[])--> doesscopehere guarantee no GC allocation at call site for all possible compiler flags? No further rewriting of function needed. Call site breakage. I.e. potential breakage of code that uses Phobos (there are a bunch of functions in Phobos that use TVF).
Is this a correct summary? (edit: formatting)
I use this too in quite a few places. And since it is not a template there's other advantages to be had, like it can be virtual (my database.d's interface uses this. a final T... template forwards to a virtual Variant[].... a design i don't like anymore but at the time - like 2011ish - it seemed elegant and it still works so I haven't changed it) and compiles quite quickly (my new static_foreach written a couple days ago uses it for this reason).
Can those be replaced with a scope array? Probably. So I think I can live without it. Though some exceptions are like simpledisplay.d uses it in an output range function which cannot be made into an array. But perhaps could just offer an overload or something.
nanovega.d uses it in a port of a C library that used to be a C variadic but this adds type safety without breaking anything. Again I could probably change it to an array I think. It'd be annoying but I think it should work the same way and only be a mild annoyance in syntax.
Though I probably use every bit of D somewhere. I know the spec well and have personally written hundreds of thousands of lines. I often find myself annoyed when these things are annoyed - just yesterday, for example, the overloaded new allocator function being removed annoyed me. (I didn't want to use it, I wanted to @disable it. settled for just putting in the docs "plz don't new this". ugh.)
- it is rarely used
I've seen it used at Sociomantic quite a bit. A quick grep on Ocean returns over 25 occurences. Haven't looked at other libraries, but it is definitely used.
Removing old features that don't agree with new or introducing ones should not be the first port of calling if we hit upon stumbling blocks though.
I'd rephrase that as old features that are redundant with new ones should be removed.
lazy parameters are still not interoperable with @safe code - there have been bug reports.
Yes, and lazy will getting the boot as being redundant and buggy.
A quick grep on Ocean returns over 25 occurences.
This PR found 4 in druntime, which took a few minutes to change.
Anyhow, I anticipate a long deprecation period.
Anyhow, I anticipate a long deprecation period.
Remember that deprecations are as bad as errors (-de). Because Phobos uses this in a number of places, it will trigger deprecations in user land, and worse, it may trigger deprecations in Phobos code.
Because this is used by dlang itself, I think it makes much more sense to prove to ourselves that indeed we don't need this, by removing our own cases first (this means API deprecation wait time first, before deprecating the feature).
Does this PR affects API for static arrays like void fun(T, size_t N)T(size_t[N] a...)?
Just as a note, as I'm not sure I care too much about having to enclose things in brackets, but I used the fact that you can overload foo(int[] arr) with foo(int[] arr...) in dcollections to know when I had to make a copy of the data and when I didn't.
Will you be able to overload foo(int[] arr) with foo(scope int[] arr)?
A further note -- many people are used to [1, 2, 3] being an allocation. I think a lot of this angst will ease once we get used to the idea that it doesn't always mean that.
A further note -- many people are used to
[1, 2, 3]being an allocation. I think a lot of this angst will ease once we get used to the idea that it doesn't always mean that.
The uncertainty means that in practice I would always assume the worst. If you cannot rely on it being heap allocated or not, there is no point: the people that care about it want to be sure, independent of compiler version or optimization flavor; the people that don't care about it, well, they don't care ;-)
So for me the requirement of using [ ] is a step back.
@WalterBright : I believe this gives us enough evidence that they are, in fact, much more used than originally thought.
If you cannot rely on it being heap allocated or not,
You can rely on it the same way the test was done for it - add @nogc. Since that specifically is in the test suite, it is reliable.
Will you be able to overload foo(int[] arr) with foo(scope int[] arr)?
No. There are various practical problems with that, not the least of which is scope inference would become hopeless.
I believe this gives us enough evidence that they are, in fact, much more used than originally thought.
Yes. But that leaves a couple issues:
-
is it actually needed?
-
the "class constructor" form of it nobody has mentioned. I presume nobody uses it?
2. the "class constructor" form of it nobody has mentioned. I presume nobody uses it?
The fact that I have to look this up (going right now) means I'm pretty sure the answer is yes ;)
Edit: oh wow yeah, I'm fine if you get rid of that. I can't even think of a really good reason for that one.
On Wed, May 13, 2020 at 12:43:29PM -0700, Walter Bright wrote:
- the "class constructor" form of it nobody has mentioned. I presume nobody uses it?
I have tried to use that for things in the past and found it wasn't really a help anyway so I'd be ok seeing it go.
Now if it worked on structs then that might be different!
- is it actually needed?
I'm going to say it would be better to keep it. It is nice to know that a certain call is not going to allocate for an array, even in a non-@nogc function. The alternative "unsafe" variadics are not great to deal with (if you want the same API). I use it not frequently, but in a lot of places it makes sense.
If there was a literal you could use for a static array that was distinct from an allocating literal, and have that automatically convert to a scope dynamic array, that would be ideal.
The uncertainty means that in practice I would always assume the worst.
This is a good point. In addition, the scope on the array could be inferred, which means that you can't even figure out easily whether it will allocate or not.
the scope on the array could be inferred, which means that you can't even figure out easily whether it will allocate or not.
If you need it on the stack, add @nogc annotation.
But what if you just want that parameter to be @nogc? Making a @nogc escape is starting to sound quite a bit more cumbersome.