Improve error supplemental

[ljmf00]

No description provided.

[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

[ljmf00]

Suggestion: we could make a pre pass of the testsuite where it instruments the code to add correct line numbers, so the initial comment doesn't affect it,

or:

Start enforcing a #line directive to most of the tests.

[dkorpel]

I want to do away with #line directives since they make it hard to locate the line the error belongs to. Instead, I'm working on implementing this: #14515

[ljmf00-wekaio]

See my comment on #14515 .

[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:.