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

Core Gradle header dependencies doesn't account for headers that can be included multiple time #861

Open
lacasseio opened this issue May 5, 2024 · 1 comment

Comments

@lacasseio
Copy link
Member

In Gradle's IncrementalCompileFilesFactory, cycles (aka already visited headers) are ignored. However, in preprocessor libraries, cycles may be intended. The header guards dictate if a cycle should be followed or not. Without a header guard, a header could be included multiple times with a different set of macros to resolve the header against. This may cause some headers to be omitted from the resulting graph. Detecting the header guard may be too hard given 1) the large possibility to guard headers (generally, we only think about #ifdef/#define and #pragma once, but it can be much more complicated) and 2) the fact that Gradle's header dependency is not meant to be a 1-to-1 representation of the actual header graph. We also need to avoid any possible cycle that would cause a stack overflow. However, we should account for the potential of new headers that may be resolved thanks to an updated visibleMacros. A sensible approach could be to 1) check if the cycle header provides any "new" state to resolve, e.g. if the header has an #include macro of a newly discovered macro, or we could 2) use a global approach where we aggregate resolvable expression and successfully resolve them against each discovered macros. Note the latter approach differs widely from the normal header resolution process, but let's keep in mind that Gradle's goal is not to make 1-to-1 the compiler's behaviour but to provide a good enough (performance and accuracy-wise) representation of the up-to-dateness and cache key for a compile task.

@lacasseio
Copy link
Member Author

Actually, the cycle detection is only meant when the file is currently being visited (the result being calculated). If the result is available, the visibleMacros will be merged. So, the behaviour should be close to option 2. More documentation should be written about this piece of code.

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

No branches or pull requests

1 participant