Skip to content

Report correct location for undefined identifiers in function return types #21223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 14, 2025

Conversation

abulgit
Copy link
Contributor

@abulgit abulgit commented Apr 13, 2025

Fix: #21165
I use the location stored in the type node (mt.loc) rather than the function declaration location when reporting undefined identifier errors. This ensures that error messages point directly to where the invalid type is declared.
Added test fix21165.d, which verifies the corrected error location. Also Fix an Existing test fail_compilation/noreturn2.d

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @abulgit! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

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

@thewilsonator
Copy link
Contributor

Need to update fail_compilation/b23686.d too with

 fail_compilation/b23686.d(107): Error: undefined identifier `eFN1`, did you mean template `eFN0()()`?
 fail_compilation/b23686.d(107): Error: `mixin(_error_)` does not give a valid type
 fail_compilation/b23686.d(115):        while looking for match for `eload!(int, 1)`
-fail_compilation/b23686.d(121): Error: undefined identifier `FNwtf`
+fail_compilation/b23686.d-mixin-121(121): Error: undefined identifier `FNwtf`
 fail_compilation/b23686.d(121): Error: `mixin(_error_)` does not give a valid type
 fail_compilation/b23686.d(126):        while looking for match for `load!"wtf"`

@abulgit
Copy link
Contributor Author

abulgit commented Apr 14, 2025

Yes, got it, but why did the test file name itself change? What is this mixin? This test file looks a bit different from other regular fail_compilation tests. Can you please tell me why this test behaves like this?

@thewilsonator
Copy link
Contributor

No idea, but it is not completely surprising, you are changing the location of the error and the source code for mixins are formatted like that.
The mixin in question is https://github.com/dlang/dmd/blob/master/compiler/test/fail_compilation/b23686.d#L36

Can you please tell me why this test behaves like this?

Why this is affected by a change in the return type? I have no idea. Unless this also affects change in the location of parameters?

@abulgit abulgit requested a review from thewilsonator April 14, 2025 08:07
@thewilsonator thewilsonator merged commit d657667 into dlang:master Apr 14, 2025
42 checks passed
@abulgit abulgit deleted the fix-21165 branch April 15, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wrong location for errors regarding function return type
3 participants