allow default valued parameters after template variadics

[timotheecour]

Function parameters with default values are now allowed after variadic template parameters, and always take their default values. This allows using special tokens (eg FILE) after variadic parameters, which was previously impossible.

Eg:

string log(T...)(T a, string file = __FILE__, int line = __LINE__)
{
  return text(file, ":", line, " ", a);
}

assert(log(10, "abc") == text(__FILE__, ":", __LINE__, " 10abc"));

This should be preferred to the previous workaround, which caused template bloat:

string log(string file = __FILE__, int line = __LINE__, T...)(T a);

[dlang-bot]

Thanks for your pull request, @timotheecour! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	8687	Variadic templates do not work properly with default arguments

[wilzbach]

Huge fan of this and I'm impressed that's a two line change.
People have been complaining about this on the NG forever and it was cited as one of the reason why logging in D "sucks".
Also

1. You can add at least https://issues.dlang.org/show_bug.cgi?id=8687 to the fixed bugs.
2. This needs a PR to dlang.org to update the spec

CC @MartinNowak as he mentioned this on Bugzilla:

While working on limited lifetimes (@safe RC/Uniq) I ran into another use-case where a default argument is needed.

[wilzbach]

Don't import Phobos if not absolutely required and you have a strong motivation.

1. It makes the testsuite a lot slower to run (DMD has to parse entire Phobos for every test)
2. If this ever fails, it makes it a lot harder to debug into

Thus, there are many "implementations" of tuple in the testsuite (do a grep) - here's a simple one:

struct Tuple(T...){
    T expand;
    alias expand this;
}
auto tuple(T...)(T t){ return Tuple!T(t); }

[timotheecour]

done; we definitely should add that to CONTRIBUTING.md
also, maybe a linter that would show this kind of policy violation

[ibuclaw]

Style

[timotheecour]

done

[jacob-carlborg]

What happens with code like this, will it compile?

string foo(T...)(T a, string file = __FILE__) if (T.length == 1)
{
    return null;
}
foo(1, "bar");

[timotheecour]

@jacob-carlborg I added a test. it does not compile as expected (and doesn't get confused by the fact "bar" is of same type as "file").

[timotheecour]

PTAL, all comments addressed.

You can add at least https://issues.dlang.org/show_bug.cgi?id=8687 to the fixed bugs.

done

This needs a PR to dlang.org to update the spec

will do after this is merged (or do you recommend another way?)

EDIT: see dlang/dlang.org#2169

[jmdavis]

Yay! Thank you, @timotheecour for doing this.

[MartinNowak]

Just seen it in the changelog, thx.

While working on limited lifetimes (@safe RC/Uniq) I ran into another use-case where a default argument is needed.

Could be helpful would have been the better wording, mainly to pass a default allocator as last argument and keep tracking it's scopeness.

[schveiguy]

This PR changed the behavior of valid code. Even though default parameters didn't make sense (because you had to specify them), it didn't stop actual code from using them. See issue 19057.

I think the better fix would be to make default parameters actually work (and match the parameters if the trailing arguments have the same type).

In other words:

void foo(A...)(A a, int x = 1, int y = 2)
{
    writeln(a, x, y);
}

void main()
{
   foo("hello"); // 1
   foo("hello", 5); // 2
   foo("hello", 5, 6); // 3
}

Prior to this PR, 1 and 2 were errors, but 3 worked and printed "hello56"
After this PR, 1, 2, and 3 work, but print "hello12", "hello512", "hello5612" which is a change in behavior for at least one previously working call.

My expectation is that it should print "hello12", "hello52", "hello56". I don't get why parameters with default values should never match the actual arguments.

[jmdavis]

I think the better fix would be to make default parameters actually work (and match the parameters if the trailing arguments have the same type).

That would be a disaster in the making for __FILE__ and __LINE__, because it would then be trivial to accidentally match them and thus give them the wrong values as well as then be missing arguments that were then intended to be part of the variadic arguments. And I really don't see much point in even having non-default parameters after variadic parameters. You can just make them part of the variadic parameters. So, I seriously question that even supporting having non-default parameters after variadic parameters is particularly worth it, and what you're suggesting would make having parameters with default arguments after variadic parameters highly risky (not just for __FILE__ and __LINE__ but for pretty much any default arguments), whereas these changes actually made them work.

[schveiguy]

That would be a disaster in the making for __FILE__ and __LINE__

No, it wouldn't. You just wouldn't do that, until we can figure out a better way (see issue 18919). Just go back to the templated versions for now.

And I really don't see much point in even having non-default parameters after variadic parameters. You can just make them part of the variadic parameters.

It's pretty obvious to me, to cut down on complexity and template bloat. What's easier/more understandable?

void foo(A...)(A args, int timeout)
// or
void foo(A...)(A args) if (A.length > 0 && is(A[$-1] == int))
{
    int timeout = a[$-1];
}

It's not for us to question how people have written their code. __FILE__ and __LINE__ are not the only use cases for default parameters, and those use cases don't always match the pattern that __FILE__ and __LINE__ do.