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

Compilation Error with includes #138

Closed
zlnimda opened this issue Jan 24, 2022 · 2 comments · Fixed by #167
Closed

Compilation Error with includes #138

zlnimda opened this issue Jan 24, 2022 · 2 comments · Fixed by #167

Comments

@zlnimda
Copy link

zlnimda commented Jan 24, 2022

Hello,

It seems compilation shaders with includes that contains forward declarations does not work as expected.

Steps to reproduce:

  • Create a include shader with a function that use another function forward declared in this shader
  • Define the forward declared function in your main shader

Result expected: no error, should be working

Current result: error message Shader failed to compile - <your included shader>

It seems your extension is trying to compile the included shader independently from what could actually include it.
At the contrary, includes could only contains snipped of code that other shaders would just include. This break the design and purpose of includes in a general way.

For exemple if I'm preprocessing my shaders with mcpp (a common preprocessor), and then just use the single shader result produced (the code is just inlined): shader works and produce no error of compilation.

@Malacath-92
Copy link
Collaborator

Malacath-92 commented Jul 16, 2023

Compiling included files is intentional, it allows us to diagnose exactly where an issue occurs without having to do more complicated line mappings. When we added this I was thinking from a C++ programmer's perspective where a scenario that you describe is kinda bad engineering, as your code written in an include suddenly depends on client code. But I don't know if it's common in the graphics world. So I'll add a setting to skip this extra step, but you can expect worse diagnostics if your included files contain errors.

@zlnimda
Copy link
Author

zlnimda commented Sep 19, 2023

Thank you very much for the recent update.

Just a little thing about the "bad engineering" point of view of yours:
I don't know much what's your experience in cpp but I will guess you already used headers and inlines.
You know that in cpp you can use forward declaration which will produce undefined symbols which will be resolved at link.

Sadly in graphics programming, you often have to compile one unit directly to your program and not assemble multiple compile units into one at a link step. That's why using forward declare does not really work in this case, they expect that your compile unit contains no unresolved symbols. Except when all includes are contained into one unit which will be compiled and resolve the forward declaration in that same unit.

You might think this is bad engineering, but it's common engineering to use such forward declaration with definition in multiple compil units. (especially in cpp: all declarations in headers does not have symbols resolved in all compile unit of each cpp file, they are only resolved at link step)

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 a pull request may close this issue.

2 participants