Bug 19057 – 2.079 changelog variadic template and default arguments

The changelog of 2.079 says: " 3. Function parameters with default values are now allowed after variadic template parameters Function parameters with default values are now allowed after variadic template parameters and when IFTI is used, always take their default values. ... " However, parameters with default values were already allowed after variadic template parameters. And the 2.079-change of behavior results in compile errors (in the best case), but may also lead to bad runtime behavior of previously working code. I hope the change was made knowingly that it was a semantic change, instead of an addition to the semantics... Simple test case: ``` import std.stdio; void bar(Args...)(Args _args, int timeout = 0) { pragma(msg, typeof(_args)); writeln(_args, timeout); } void main() { bar("a", "b", 2); // used to print "ab2", but since 2.079 prints "ab20" } ``` Semantic changes like this need to be very clearly marked in the changelog. Updating the compiler on a commercial codebase is already enough of an adventure, no need to add extra uncertainties. Please fix the changelog. Thanks, Johan

IMO, this should either be reverted or go through a deprecation cycle. I'm not seeing what the pros/cons are of each mechanism. Is there a discussion somewhere?

It appears the PR that implemented this was https://github.com/dlang/dmd/pull/7831 The changelog was then amended in https://github.com/dlang/dmd/pull/7875 A spec change was also made in https://github.com/dlang/dlang.org/pull/2169

So the use case for the new style is pretty well discussed and documented (and makes a lot of sense). The use cases for the old style are not discussed really. Looks like the only times default parameters were used is when the number of parameters was less than the default (then the variadic template was inferred as an empty list). I can potentially see the benefit there. Can that functionality be duplicated with the new mechanism? That is the main question we have to answer. If it can, then the deprecation period can be relatively short, with a nice message on how to rewrite. If not, then I don't know what the right path is. Clearly it's a hugely useful thing to be able to specify default parameters for line and file without doing it in the template list. Johan, do you have some sample code to show a good use case for the old way?

My concern is not about use cases. It's about breaking old code (silently), and also about strange (arguable) language behavior in the current situation. Note that before 2.079, the default argument was never used: the template would not match (in the testcase, `bar("a","b");` would not compile pre-2.079). (which is probably why Timothee wrote that it now becomes possible to use default args) Breaking old code: The fact that default argument was never used in pre-2079 is bad news: it means that old code always specifies a value for the default value, and thus that with the new compiler things are always broken (except when the caller is not using IFTI, which is unlikely I think). Strange new behavior: Adding a default value for a parameter now changes the template instantiation (if IFTI is used, which I think will happen in the majority of cases). ``` // Adding the default value will change behavior. void bar(Args...)(Args _args, int timeout /* = 0 */) { writeln(_args, timeout); } void main() { bar("a", "b", 2); // used to print "ab2", but since 2.079 prints "ab20" } ``` How many people would expect that to happen? The use case of the new style does not discuss much about how to override the default value (something that is trivially explained for normal functions). Here, you need to disable IFTI, which is painful. I've mainly seen discussion of the use case with __FILE__, but not for the general case. Note: there is no old use case, because the default value itself didn't work (syntax was allowed, but always need to specify value for the parameter at call site). A better fix for the __FILE__ use case would perhaps have been something like this: `void bar(Args...)(Args args, TypeThatIsNeverMatched = TypeThatIsNeverMatched.init, string file = __FILE__)`, where default arguments work but IFTI matching on callsite first matches the non-variadic parameters, and then starts matching the variadic part.

Honestly, I would guess that far more code has been written to take advantage of the new behavior since it was released than was ever purposefully written to use the old behavior. I think that many (most?) of us had no clue that there was any way to use default arguments with variadic templates and IFTI prior to the 2.079 release, and many were very interested in the new behavior, because it fixes the file and line problem. So, changing the behvior back (as would have to be done to deprecate it) would definitely break existing code (whereas I question that much code was broken with the change in 2.079), and we've now had three major releases with the new behvior. So, I don't think that it makes any sense to try and deprecate the old behavior at this point. Clearly, the fact that this was a breaking change was not properly caught, and we should have deprecated the old behavior before introducing the new. But while that was definitely a screw-up, I don't see how it makes much sense to backtrack now. The new behavior has been out for three releases and is something that enough folks were looking for that I expect that there are quite a few folks using it now. So, if anything, if we were to decide that the current behavior is undesirable, then we arguably need a deprecation warning for that, not the old behavior.

OK, I misunderstood. So essentially, any function that uses variadic parameters AND a normal parameter with a default value was EXACTLY the same as a function which takes the varargs parameter and a normal parameter WITHOUT a default value prior to 2.079. i.e. ALL code that uses default parameters after varargs parameters can be rewritten to simply remove the default parameter, and the same behavior occurred But in addition, there is a strange new behavior that has default parameters NEVER match the parameters unless you disable IFTI. Indeed I find it odd that this would be the case: void foo(A...)(A a, int timeout) {} void bar(A...)(A a, int timeout = 0) {} foo(1); // equivalent to foo!()(1) bar(1); // equivalent to bar!(int)(1, 0) ??? Given the use case of file and line parameters, this makes sense, as you almost never want to pass those parameters. But in the general case it makes very little sense. I'm pretty sure phobos was changed to take advantage of this immediately. So reverting would be really painful, right? If I were to start over before 2.079, this is the path I would have taken: 1. Make default parameters in these cases actually work in the way we would expect (i.e. they match if they are the right type, otherwise they go to the variadic). 2. Fix issue 18919 3. Create a new type `Location`, which accepts a filename and line number, and use THAT as the final parameter for such functions, with a default value of Location loc = Location(__FILE__, __LINE__). Now, you would never "accidentally" pass in a file or line number. I don't know the path forward now...

Thanks for the support Steven. <rant> I hope this helps put the message across to be _very careful_ in making language additions / changes. This should have been a DIP. And not a PR that gets merged after a single (!) day. It's not a good story at all for industrial use of D. Note that with the current high-paced release cycle, testing of new compilers is guaranteed to happen only after 2 new releases have happened, as I've argued before. Proof given again in this bug report. This did break code at Weka, luckily in non-silent way, but now I probably will have to modify the old compiler to error on these things, to not cause trouble with silently broken code. Regardless of breakage, the new behavior is also one more special case to learn about in the language. I can already see style guides telling people to never use this functionality, except for __FILE__. This is a bit of a rant, I know, my apologies for that, but it seems my message has not gotten through yet and I don't know how else than to just repeat it. </rant>

I think we need to find a way to change this behavior, and soon. As it stands now, it doesn't make sense, as it's only useful in the __FILE__ and __LINE__ context. It could easily be generalized to fit all contexts, even in a way where the original behavior is valid. I'd flag this as a regression if it were up to me. The change that allows default parameters to work is reasonable. The change that makes them NEVER match IFTI-passed arguments is very bad and not intuitive. I'd absolutely expect this to work like anyone would think it should: Exception genErr(A...)(A args, string file = __FILE__, size_t line = __LINE__) Exception genSpecificError(string file = __FILE__, size_t line = __LINE) { return genErr(1, 2, 3, file, line); } And the workaround is NOT pleasant.

(In reply to johanengelen from comment #7) > This did break code at Weka, luckily in non-silent way, but now I probably > will have to modify the old compiler to error on these things, to not cause > trouble with silently broken code. I'm curious if there is any reason to have default parameter specification in any of these cases. Given that you could never use the default parameters anyway (you had to specify them all), isn't the fix to just change them all to not have default values?

(In reply to Steven Schveighoffer from comment #9) > (In reply to johanengelen from comment #7) > > This did break code at Weka, luckily in non-silent way, but now I probably > > will have to modify the old compiler to error on these things, to not cause > > trouble with silently broken code. > > I'm curious if there is any reason to have default parameter specification > in any of these cases. Given that you could never use the default parameters > anyway (you had to specify them all), isn't the fix to just change them all > to not have default values? Indeed, the fix is trivial. But finding all places to fix it may not be so easy. Perhaps it's not so bad and a regexp search will find them. I was happy to find out that it never really worked.

(In reply to Steven Schveighoffer from comment #8) > I think we need to find a way to change this behavior, and soon. As it > stands now, it doesn't make sense, as it's only useful in the __FILE__ and > __LINE__ context. It could easily be generalized to fit all contexts, even > in a way where the original behavior is valid. I'd flag this as a regression > if it were up to me. > > The change that allows default parameters to work is reasonable. The change > that makes them NEVER match IFTI-passed arguments is very bad and not > intuitive. > > I'd absolutely expect this to work like anyone would think it should: > > Exception genErr(A...)(A args, string file = __FILE__, size_t line = > __LINE__) > > Exception genSpecificError(string file = __FILE__, size_t line = __LINE) > { > return genErr(1, 2, 3, file, line); > } > > And the workaround is NOT pleasant. Well, having file or line end up being given values just because a string or string and integral value happened to be last in the argument list would arguably be really bad, because then it becomes trivial to screw up the file and line number, whereas if you have to explicitly instantiate the function template to give file and line different values, then we don't have that problem, and in most cases, explicitly passing values for file or line is unnecessary, so arguably the extra annoyance isn't likely to come up often enough to be a big deal. I confess that I never would have expected parameters to be allowed after variadic arguments though, so the current behavior actually matches up extremely well with what I would have expected the behavior to be if we allow paramters with default arguments after the variadic parameter.

(In reply to Jonathan M Davis from comment #11) > Well, having file or line end up being given values just because a string or > string and integral value happened to be last in the argument list would > arguably be really bad, because then it becomes trivial to screw up the file > and line number, whereas if you have to explicitly instantiate the function > template to give file and line different values, then we don't have that > problem, and in most cases, explicitly passing values for file or line is > unnecessary, so arguably the extra annoyance isn't likely to come up often > enough to be a big deal. That is already a problem without variadics. For example, if you change the parameters of a non-variadic function to insert a string before the __FILE__ default, now you have code that may still compile even with the old version and not do what it's supposed to do. Part of the problem for this pattern is that the type of the __FILE__ and __LINE__ parameters are normal types used all over the place (string and size_t). That is not the fault of variadics, but the fault of the pattern. It's why we have things like enums. The real solution is to make it so you can pass in a converted type with the __FILE__ or __LINE__ as the default, but this doesn't work (see issue 18919). If that was fixed, then we can change all the file/line parameters to something more explicit than string/int. See this relevant discussion: https://forum.dlang.org/post/[email protected] > I confess that I never would have expected parameters to be allowed after > variadic arguments though, so the current behavior actually matches up > extremely well with what I would have expected the behavior to be if we > allow paramters with default arguments after the variadic parameter. But that was never in question. Variadics with trailing parameters was always allowed. It's that if you had default values, the default values were NEVER USED. The problem with the PR that caused all this mess is that it changes the behavior of those parameters when you give it a default, and this was a silent breaking change. If the compiler simply rejected those parameters to begin with, then this would have been less of a problem.

(In reply to Jonathan M Davis from comment #11) > > I confess that I never would have expected parameters to be allowed after > variadic arguments though, so the current behavior actually matches up > extremely well with what I would have expected the behavior to be if we > allow paramters with default arguments after the variadic parameter. That argument only makes sense iff default arguments are obligatory for parameters after variadic parameters. But that's not the case, default arguments are _not_ required and that has been working for a long time. The current behavior only makes sense in isolation, but breaks when considering slightly larger scope (i.e. with _and_ without default argument specified).

The current behavior works well with default arguments, because then they never get accidentally matched, whereas IFTI is allowed to match arguments against them, then it becomes trivial to match them accidentally, and if that were the case, I'd argue strongly that putting default arguments on parameters that come after variadic parameters was too error-prone for it to make any sense. At least in the case where there are no default arguments, you always have to provide the arguments, so it's expected. But once you have default arguments, those arguments are no longer expected, and it becomes trivial to think that you're passing an argument that matches the variadic parameters but ends up matching at least one of the parameters after the variadic parameters. The behavior would be fatal for any attempt at something like logging because of how much would accidentally match against __FILE__, but any case where you have parameters with default arguments following varidiac parameters would be very error-prone unless the types involved were extremely unlikely to be passed to the function. I fail to see why having the parameters with default arguments matching with IFTI would ever be desirable. It's just too error-prone.

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dlang.org/issues/4090 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB