Skip to content

Overhaul DefaultInitExp#22903

Merged
dkorpel merged 5 commits intodlang:masterfrom
dkorpel:defaultarg-syntaxcopy
Apr 10, 2026
Merged

Overhaul DefaultInitExp#22903
dkorpel merged 5 commits intodlang:masterfrom
dkorpel:defaultarg-syntaxcopy

Conversation

@dkorpel
Copy link
Copy Markdown
Contributor

@dkorpel dkorpel commented Apr 9, 2026

Big PR, will split off parts but first see how test suite reacts.

Probably fixes (edit: not yet) #18670 (needs tests), supersedes #14309

@dlang-bot dlang-bot added the WIP label Apr 9, 2026
@dlang-bot
Copy link
Copy Markdown
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! 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#22903"

@dkorpel
Copy link
Copy Markdown
Contributor Author

dkorpel commented Apr 9, 2026

/buildkite/builds/geod24-main-3/dlang/dmd/buildkite-ci-build/distribution/bin/../imports/std/process.d(798,41): Error: `@safe` function `test.executeCommand` cannot call `@system` function `std.stdio.makeGlobal!"core.stdc.stdio.stderr".makeGlobal`
                 File stderr = std.stdio.stderr,

Yep, that's a bug fix and a breaking change, will require either an edition guard or a deprecation.

@Herringway
Copy link
Copy Markdown
Contributor

Glad to see this hole finally getting patched!

@dkorpel dkorpel force-pushed the defaultarg-syntaxcopy branch from e35535d to 22383a6 Compare April 9, 2026 22:54
@thewilsonator
Copy link
Copy Markdown
Contributor

Test suite looking good, ping me when this is good to go.

@dkorpel dkorpel marked this pull request as ready for review April 10, 2026 12:13
@dkorpel dkorpel requested a review from ibuclaw as a code owner April 10, 2026 12:13
@dkorpel dkorpel added Severity:Refactoring No semantic changes to code and removed WIP labels Apr 10, 2026
@dkorpel dkorpel changed the title [WIP] Overhaul DefaultInitExp Overhaul DefaultInitExp Apr 10, 2026
@dkorpel
Copy link
Copy Markdown
Contributor Author

dkorpel commented Apr 10, 2026

So I marked this PR as a refactor, switching from inlineCopy + resolveLoc to syntaxCopy + expressionSemantic. In more detail:

  • Implement syntaxCopy for DefaultInitExp (the default syntaxCopy also copies the type which makes expressionSemantic quit early)
  • Remove inlineCopy for default arguments, use syntaxCopy instead
  • Remove subclasses / enum EXP members for each __FILE__ __LINE__ etc. token, and give DefaultInitExp a TOK to differentiate it instead
  • Replace the resolveLoc() visitor with doing it directly in expressionSemantic
  • Add call site location callLoc to the Scope struct, next to callsc, which expressionSemantic uses to resolve locations like resolveLoc did before.

This means less class boilerplate, not having a separate visit pass for resolveLoc anymore, and removing a weird dependency on inline.d. Since I wouldn't want to bury a bug fix in this big refactor, I disabled attribute checks for default arguments with:

    if (sc.inDefaultArg || sc.callLoc.isValid)
        return false;

In a follow up PR I plan to replace these with actual deprecation / edition logic to fix the bug, and add a test and changelog. If this all sounds good, it's ready to merge @thewilsonator

@thewilsonator
Copy link
Copy Markdown
Contributor

I'll leave it to you to squash or rebase and merge as you see fit.

@dkorpel dkorpel merged commit 3bedbac into dlang:master Apr 10, 2026
42 checks passed
@dkorpel dkorpel deleted the defaultarg-syntaxcopy branch April 10, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Severity:Refactoring No semantic changes to code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants