dmd icon indicating copy to clipboard operation
dmd copied to clipboard

Add visit method for UnionDeclaration in transitivevisitor

Open lucica28 opened this issue 3 years ago • 4 comments

UnionDeclaration should have it's own visit method because one might want to extend the transitive visitor class and override the visit method of StructDeclaration, but keep the default logic for UnionDeclaration. Without a visit method for UnionDeclaration, what would happen currently in this situation is that if you override the visit method for StructDeclaration you automatically override also the visit method for UnionDeclaration

lucica28 avatar Sep 18 '22 20:09 lucica28

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

dlang-bot avatar Sep 18 '22 20:09 dlang-bot

Small suggestion, don't merge, rebase.

ibuclaw avatar Sep 19 '22 12:09 ibuclaw

CI failure is because you need to update the C++ headers to match the D code. From the log:

The file src/dmd/frontend.h seems to be out of sync. This is likely because changes were made which affect the C++ interface used by GDC and LDC.

Make sure that those changes have been properly reflected in the relevant header files (e.g. src/dmd/scope.h for changes in src/dmd/dscope.d).

To update frontend.h and fix this error, run the following command:

src/build.d cxx-headers-test AUTO_UPDATE=1

Note that the generated code need not be valid, as the header generator (src/dmd/dtoh.d) is still under development.

To read more about frontend.h and its usage, see src/README.md#cxx-headers-test

pbackus avatar Sep 20 '22 16:09 pbackus

Small suggestion, don't merge, rebase.

thank you for the suggestion

lucica28 avatar Sep 21 '22 18:09 lucica28