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.

Sign up for GitHub

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

Jump to bottom

Circular dependency in include files #15

Open
MathiasMagnus opened this issue Aug 5, 2020 · 4 comments · May be fixed by #28
Open

Circular dependency in include files #15

MathiasMagnus opened this issue Aug 5, 2020 · 4 comments · May be fixed by #28
Labels
bug Something isn't working

Comments

@MathiasMagnus
Copy link

I'm trying to compile the library using CMake and Ninja instead of the Visual Studio generators. I was aware that IntelliSense is smarter when used in tandem with MSBuild, as it picks up global information of the build and doesn't operate solely based on translation units. When building with Ninja, after realizing that PCH isn't a build acceleration method but is actually a hard requirement of the build, I added target_precompiled_header invocation and realized that the project still doesn't build. I started adding #includes directives to the headers to please the compiler, but ultimately engaged in cyclic include directives. A lot of headers arrive to including DeviceChild.hpp which in turn requires ImmediateContext.hpp which in turn requires Resource.hpp which then requires DeviceChild.hpp again. Removing any of these includes results in undefined types when compiling the very same pch.hpp the build otherwise relies on.

Can anyone comment on how to untie this knot?
@jenatali
Copy link
Member

jenatali commented Aug 5, 2020

Are you still trying to use MSVC as the compiler, or are you trying to use a different compiler? From my skim of the code in DeviceChild.hpp, I don't see why that file would need ImmediateContext.hpp, at least with MSVC's broken template parsing. A forward declaration of the ImmediateContext class should be sufficient to satisfy any non-template code.

As I said in the readme, currently MSVC is the only compiler that we've tried to use to build this code, though given the announcement of WSL support, we do have aspirations to make it cross-platform, since it serves as the basis of the OpenCLOn12 component, which we plan to leverage in WSL.

@MathiasMagnus
Copy link
Author

MathiasMagnus commented Aug 6, 2020 •
edited
Loading

As a random thought this morning it has occured to me it is likely my CMake toolchain file causing the issue, where I enable /permissive- unconditionally for all builds (I'm a cross-platform dev) and that likely changes the semantics of template parsing, hence the forward declaration isn't enough in this mode. With WSL support in place, even though Clang may be forced to imitate MSVCs lazy template parsing, it may be a better choice forward to make it ISO C++ conformant, in case distros using GCC would actually wanted to include these packages (or parts of it) in their repos.

Edit: indeed, it was /permissive- at "fault", not adding it as a build flag allows compilation to pass. Would the project benefit from ISO C++ conformance?

@jenatali
Copy link
Member

jenatali commented Aug 6, 2020

Yeah, ISO C++ conformance is the first step to making this code cross-platform. The second step is removing Windows-only concepts and embracing std:: replacements whenever possible. Both of these are on our roadmap, and I'd be happy to accept pull requests that help us along towards that path.

@jenatali jenatali linked a pull request Nov 11, 2020 that will close this issue
Fixes to allow msvc /premissive- #28
Open
@weltkante
Copy link

It seems MSVC is moving away from permissive:

/std:c++latest now implies /permissive-. This means that customers currently relying on the permissive behavior of the compiler in combination with /std:c++latest are required to now apply /permissive on the command line.

Switching permissive on in the project options does not work anymore, it is ignored, you literally have to pass /permissive on the command line (under additional options in the command line section of the project options). Just a heads-up for anyone trying to make use of the latest C++ features while including the D3D12 translation layer headers.

@jenatali jenatali added the bug Something isn't working label May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes to allow msvc /premissive- xaphier/D3D12TranslationLayer
3 participants
@weltkante @MathiasMagnus @jenatali