Improve error supplemental

[ljmf00]

[dlang-bot]

Thanks for your pull request, @ljmf00!

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#16755"

[ljmf00]

Can we agree on a format first before I change every expcted failure test messages? Do you think these messages are ok? CC @dkorpel @JohanEngelen

[dkorpel]

I like the recent addition of supplemental error messages pointing to the declaration site of types / functions related to the error, so adding it to templates looks good to me. If I understand correctly, it just shows the first template declaration, not the 'closest' matching template, but that's a future enhancement.

I don't understand the "instantiated from here: Foo!int" supplemental error though, doesn't it always point to the same location as the initial error?

[ljmf00-wekaio]

I like the recent addition of supplemental error messages pointing to the declaration site of types / functions related to the error, so adding it to templates looks good to me. If I understand correctly, it just shows the first template declaration, not the 'closest' matching template, but that's a future enhancement.

I don't understand the "instantiated from here: Foo!int" supplemental error though, doesn't it always point to the same location as the initial error?

The first one does, but the instantiated from here: Foo!int`` is part of the instance stack print (the helper function printInstantiationTrace), when it has more than one in the chain it gives you another instantiated from here until the root instantiation. This is similar to other template instance errors that uses this.

Maybe an alternative. Instead of this:

fail_compilation/constraints_aggr.d(109): Error: template instance `imports.constraints.S!int` does not match template declaration `S(T)`
  with `T = int`
  must satisfy the following constraint:
`       N!T`
fail_compilation/constraints_aggr.d(109):        instantiated from here: `S!int`
fail_compilation/imports/constraints.d(67):        Candidate match: S(T) if (N!T)

We could do this, perhaps (notice the difference on line numbers):

fail_compilation/constraints_aggr.d(67): Error: template instance `imports.constraints.S!int` does not match template declaration `S(T)`
  with `T = int`
  must satisfy the following constraint:
`       N!T`
fail_compilation/constraints_aggr.d(109):        instantiated from here: `S!int`

Maybe rewrite the main error message but I'm out of ideas other than swapping it:

fail_compilation/constraints_aggr.d(67): Error: template declaration `S(T)` does not match template instance `imports.constraints.S!int`

[ljmf00-wekaio]

I like the recent addition of supplemental error messages pointing to the declaration site of types / functions related to the error, so adding it to templates looks good to me. If I understand correctly, it just shows the first template declaration, not the 'closest' matching template, but that's a future enhancement.

It should show up on the multi candidate error message. Maybe I'm missing what you meant. Can you elaborate?

[ljmf00]

Ping @dkorpel

[dkorpel]

I mean when there's multiple template overloads, which candidate(s) does it print? My impression was it just picks one, but I didn't study it closely yet.

[ibuclaw]

I don't see any tests that use -vcontext - can at least one test be added so that the effect on this diagnostic switch can also be seen/checked?

[ljmf00-wekaio]

I mean when there's multiple template overloads, which candidate(s) does it print? My impression was it just picks one, but I didn't study it closely yet.

See this test example:

fail_compilation/fail134.d(105): Error: template instance `foo!(f)` does not match template declaration `foo(T)`
fail_compilation/fail134.d(105):        instantiated from here: `foo!(f)`
fail_compilation/fail134.d(106):        instantiated from here: `bar!(f)`
fail_compilation/fail134.d(104):        Candidate match: foo(T)
fail_compilation/fail134.d(105):        `f` is not a type

[dkorpel]

There's still just one template overload there. What I'm wondering about is this:

void foo(int n : 0) {}
void foo(int n : 1) {}
void foo(int n : 2) {}

alias foo3 = foo!3;

But I noticed that it already prints all candidates in that case:

onlineapp.d(6): Error: template instance `foo!3` does not match any template declaration
onlineapp.d(6):        Candidates are:
onlineapp.d(2):        foo(int n : 0)()
onlineapp.d(3):        foo(int n : 1)()
onlineapp.d(4):        foo(int n : 2)()

The error this PR improves is when it already found a template it must match, but there's an error in the template body. If that's correct, then I think the error should simply say: "Template declared here:", because "Candidate match: " suggests that it prints only one of multiple candidates.

[ljmf00-wekaio]

There's still just one template overload there. What I'm wondering about is this:

void foo(int n : 0) {}
void foo(int n : 1) {}
void foo(int n : 2) {}

alias foo3 = foo!3;

But I noticed that it already prints all candidates in that case:

onlineapp.d(6): Error: template instance `foo!3` does not match any template declaration
onlineapp.d(6):        Candidates are:
onlineapp.d(2):        foo(int n : 0)()
onlineapp.d(3):        foo(int n : 1)()
onlineapp.d(4):        foo(int n : 2)()

The error this PR improves is when it already found a template it must match, but there's an error in the template body. If that's correct, then I think the error should simply say: "Template declared here:", because "Candidate match: " suggests that it prints only one of multiple candidates.

ok, makes sense, will change to Template declared here:.