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

Allow to include! generated code #20

Open
9SMTM6 opened this issue Jun 25, 2024 · 2 comments
Open

Allow to include! generated code #20

9SMTM6 opened this issue Jun 25, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@9SMTM6
Copy link

9SMTM6 commented Jun 25, 2024

This is somewhat more annoying. If I want to include! the generated code - which is required for the usual strategy that moves the generated source into the out directory in the build script - , then this wont work, because of the root level allow #![allow(unused, non_snake_case, non_camel_case_types, non_upper_case_globals)]. Rust doesn't accept these in an include!, even if copy pasting the code would've been accepted in that spot.

With the current structure this should be addressable by moving these into the generated modules instead of at root level. However that would mean a commitment to not doing these things at top level, at least without dedicated allow attribute.

@Swoorup
Copy link
Owner

Swoorup commented Jun 27, 2024

I'll have to review each of these cases. IIRC those casing issues arises if some shaders don't confirm to rust naming convention standards. It was easier to keep everything in sync, hence just relaxing some of the rules in rust.

Perhaps all allows could be made optional in the option. But you still have to specify them elsewhere though. I haven't tested using include! though.

@9SMTM6
Copy link
Author

9SMTM6 commented Jun 27, 2024

I mean, as I said, with the generated code I've seen so far, you could just move these definitions into each module generated from a shader.

Thats fine with include!, it just doesn't like to have these definitions at root level (of the included code, meaning it would connect to the place it's included in, and that isn't really clean macro use, which is I suppose why include! doesn't like it).

When I made this report it was just going of a general stomache ache with having generated code in src, but now I've seen actual issues caused by this.

If you change the generated code based on crate features, then this can cause issues with incompatible code generated between different invocations of build.rs.

In particular in my case, I have serde behind a flag, since including it leads to a huge increase in WASM size. That flag is enabled by default, but disabled for my web builds. But now I often get compiler errors if I have the project open in VSCode with rust analyzer, and build for the web, since sometimes VSCode triggered a build.rs invocation with default features, meaning that serde is used in the generated code, the same code that my web build is using without regenerating, since as far as it knows, everything is fine.

If I were able to generate the code in the out directory, like how wgsl_to_wgpu does recommend in its Readme at the end, then this would be fine, since the web and the native build use different targets, and thus different out directories.

But that example uses include!.

Mind you, it's still manageable. I just need to manually trigger a rebuild of the generated code. But still, it's a real reason that being able to include! would be nice.

@Swoorup Swoorup added the enhancement New feature or request label Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants