Skip to content
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

Convert DolphinTool to modules #11057

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Zopolis4
Copy link
Contributor

This serves really more so as a demo to how to convert to modules:

  • Wrap all includes in a global module fragment
  • Add any includes in the header to the implementation file
  • Add any variables in the header to the implementation file
  • Export the functions declared as public in the header

A bit of module discussion:
Using modules will on the whole remove the need for precompiled headers, as there are no headers. We should also move to C++23 when that is finished, as it will provide a standard way to import the stdlib, which will also cut out a lot of the need for PCH. While we currently have to wrap most of the file in a namespace block, I am working on a paper that will remove the need for this. If When it gets accepted, it will be into C++26, so I propose when that happens we move to C++26, but do not use any additional features until it becomes finalized.

@Zopolis4
Copy link
Contributor Author

Zopolis4 commented Sep 12, 2022

Ok linux builders are failing due to pulseaudio skill issues, no idea what windows is doing. I'd hoped to avoid the /experimental:module flag because that enables a wide variety of nonstandard behaviour, but I guess I'll give it a try.

@Minty-Meeo
Copy link
Contributor

One thing worth considering: we should avoid making difficult-to-follow blame history when converting header-only Common files to *.cpp file extensions. In other words, do whatever makes the change register as a file being moved vs one being deleted and another being added (my inexperience with Git limits me from saying what, exactly).

@Zopolis4
Copy link
Contributor Author

The change is, to my knowledge, defined by how much you change the file and what git counts as a modification. You could also split it into two commits if you end up changing it enough to trigger git.

@Zopolis4
Copy link
Contributor Author

Zopolis4 commented Sep 12, 2022

Ah-- this is breaking on the windows builders because Visual Studio wants module code to be in .ixx or .cppm files. I'll look into a CompileAs option.

@Zopolis4
Copy link
Contributor Author

This is also failing, because, along with our buildbots being a bit behind, CMake doesn't have good module support in larger projects. See issue.

@Zopolis4
Copy link
Contributor Author

CMake is getting better-- I'll wait until an actual numbered release is pushed out and the gcc and clang stuff is merged upstream, but its a lot closer than it was. See issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants