Skip to content

Refactor: Move resolvedLinkage from AST to Semantic layer#22597

Open
Adityazzzzz wants to merge 1 commit intodlang:masterfrom
Adityazzzzz:refactor-resolved-linkage
Open

Refactor: Move resolvedLinkage from AST to Semantic layer#22597
Adityazzzzz wants to merge 1 commit intodlang:masterfrom
Adityazzzzz:refactor-resolved-linkage

Conversation

@Adityazzzzz
Copy link
Contributor

This PR moves the resolvedLinkage logic from dmd.declaration.Declaration to dmd.dsymbolsem.
As part of the ongoing effort to separate semantic routines from AST nodes, resolvedLinkage was identified as an "intricate" dependency. While it appeared to be a simple property, it performed target-specific resolution (target.systemLinkage()), creating a semantic "leak" in the AST layer.

Changes

  • declaration.d: Removed final LINK resolvedLinkage() from the Declaration class.
  • dsymbolsem.d: Introduced a new semantic routine LINK resolvedLinkage(const scope Declaration d) to host the resolution logic.
  • func.d: Updated isMain, isCMain, isWinMain, isDllMain, and isRtInit to use the new semantic routine.
  • mangle/package.d & dtoh.d: Updated the mangler and header generator to call the semantic resolution instead of the AST property.
  • glue/: Synchronized the backend glue code (toobj.d, tocsym.d, package.d) with the new architectural separation.

Impact
This change decouples the Declaration AST node from the target configuration and allows func.d to move closer to the "Done" list in the "Separate Semantic Routines" migration tracker.

This decouples the Declaration AST node from target-specific semantic checks and enables further cleanup of func.d and the backend glue code.
@dlang-bot
Copy link
Contributor

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

@Adityazzzzz
Copy link
Contributor Author

Adityazzzzz commented Feb 20, 2026

@RazvanN7 I've started tackling the 'intricate solutions' for the Separate Semantic Routines project as part of my GSoC 2026 preparation.
This was one of the intricate cases involving backend interaction you mentioned in the project description. I'd appreciate any feedback on whether this provider-based approach aligns with the long-term architectural goals.

return true;
}

extern (C++) LINK resolvedLinkage(const Declaration d)
Copy link
Member

@ibuclaw ibuclaw Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this in cxxfrontend.d (and appropriate C++ header under the correct namespace). The top-level function in dsymbolsem should be extern(D).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also link test in compiler/src/tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All C++ consumers (gdc and ldc) should access exposed routines via the dmd namespace - for this, dmd::resolvedLinkage(d).

Copy link
Member

@ibuclaw ibuclaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a little reflection, Is this is a good example to move? It doesn't do any semantic mutation on the decl. The main reason for splitting out methods from the tree.

@Adityazzzzz
Copy link
Contributor Author

@ibuclaw thank you for the review,
Regarding your reflection on whether this is a good candidate: That is a very clarifying distinction. My initial thought process for moving it was to decouple the Declaration AST node from the global target state (target.systemLinkage()). However, I completely agree with your point that it doesn't perform any semantic mutation on the tree, which is the primary goal of the separation project.
I was trying to ensure I strictly align with the gsoc project's boundaries.

However, would you prefer I to?

  • Implement your feedback here (moving the bindings to cxxfrontend.d, adding the dmd:: namespace, and writing the link test) just to practice the C++ interoperability workflow?
  • Or, should I close this PR and pivot my focus to a function that performs actual semantic mutation (e.g., migrating a mutating pass from dstruct.d or dtemplate.d)?

Either way, your guidelines regarding cxxfrontend.d are incredibly valuable for mapping out.

@ibuclaw
Copy link
Member

ibuclaw commented Feb 20, 2026

Have a look at the cxxfrontend module - what it's doing (its just a simple wrapper around extern(D) symbols), and run git blame on it to see what previous additions to it looked like.

@ibuclaw
Copy link
Member

ibuclaw commented Feb 20, 2026

I think some easy examples to get started would be any of the extern (D) static member functions currently residing in AST node definitions. combine or extractLast?

@Adityazzzzz
Copy link
Contributor Author

thank you @ibuclaw, I will look into how cxxfrontend.d wraps extern(D) symbols using git blame to understand the pattern.
I'll pivot away from resolvedLinkage and start working on extracting combine or extractLast as my starting point for the AST separation. I'll open a fresh PR for that once I have it mapped out

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.

3 participants