-
Notifications
You must be signed in to change notification settings - Fork 94
feat: Add Darwin-style framework search with ordered path lookup #511
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
base: master
Are you sure you want to change the base?
Conversation
This should be further improved following guidance originally posted by @glankk in #448 (comment)
|
Looks like this is currently executed for all code even if there is no apple framework is available which seems excessive. I wonder if this behavior should be configurable via DUI or consider the Even if it is available it might not be the files you are looking for (can there be conflicts?) as this behavior would be limited to the Apple compiler. |
Indeed, this was a deliberate choice anticipating the use of the tool in the context of cross compilation. Adding an option to enable/disable the behavior may be best. |
c5a5cbe
to
cdd5fc2
Compare
@glankk Thanks for the suggestion 🙏 All the review comments have been addressed. Once the GitHub workflow have been enabled and the test pass, I believe this will be ready for integration 🚀 we also included a comprehensive docstring explaining how to work with the
To improve the developer experience, we could also further extend the
If that sounds reasonable, I can amend the last commit and update the tests. cc: @danmar |
Waiting this is integrated, we will stage those changes into a fork and move forward with vendoring those into Related: |
#283 possibly needs to be addressed as prerequisite of this as the include directories are currently not differentiated between system and "local" ones. |
Thanks again @hjmjohnson for working on the initial patch and establishing the momentum 🙏 Hopefully our contributions will be integrated shortly 🤞 |
39eeb91
to
e0ed0e3
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Apple frameworks store headers under: <Pkg.framework/Headers/MyHdr.h> This patch extends `openHeader(...)` so that `__has_include(<Pkg/MyHdr.h>)` resolves to the framework layout. A new test `appleFrameworkHasIncludeTest` verifies the behavior. Tests: - Add dummy `Foundation.h` fixture under `testsuite/Foundation.framework/Headers/`. Note: this applies only to `__has_include`; plain `#include` still uses the existing lookup logic. Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
Follow-up to the earlier patch that added framework handling for `__has_include`. This change updates `preprocess()` so that plain `#include <Pkg/MyHdr.h>` also resolves to `<Pkg.framework/Headers/MyHdr.h>` when present. Tests: - Add `appleFrameworkIncludeTest` to verify `#include` resolution. Co-authored-by: Hans Johnson <[email protected]>
e0ed0e3
to
3a900a2
Compare
__has_include
and #include
…th lookup This change teaches simplecpp to resolve headers from Apple-style Framework directories while preserving the left-to-right order of interleaved -I/-F/-iframework search paths (like GCC/Clang on Darwin). - Add `DUI::SearchPath` with `PathKind {Include, Framework, SystemFramework}`. - If `DUI::searchPaths` is non-empty, use it verbatim (interleaved -I/-F/-iframework). Otherwise preserve back-compat by mirroring `includePaths` as Include paths. - Update `openHeader()` to consult typed paths, and only rewrite `<Pkg/Hdr.h>` to `Pkg.framework/{Headers,PrivateHeaders}/Hdr.h` when a package prefix exists. - Implement `toAppleFrameworkRelatives()` returning prioritized candidates (Headers first, then PrivateHeaders). - Update CLI: support `-F<dir>` and `-iframework<dir>` (and keep `-I` as before). - Tests use `PathKind::Framework` when checking framework layout. Behavior notes - The order of -I/-F/-iframework is preserved exactly as provided. - `Framework` vs `SystemFramework` differ only in diagnostic semantics (not lookup). - Legacy users who only set `DUI::includePaths` see identical behavior. This brings simplecpp closer to GCC/Clang behavior on macOS and enables robust resolution of framework headers like `Foundation/Foundation.h`. Suggested-by: [email protected]
3a900a2
to
6899f8e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I understand it correctly.. if the command line only uses -I
then behavior will be unchanged?
this looks pretty good to me. I would like a review by @glankk
To improve the developer experience, we could also further extend the DUI API:
please look at adding such helper functions. It sounds good to me.
This PR enhances simplecpp to support resolving headers from Apple-style Framework directories while preserving the left-to-right order of interleaved -I/-F/-iframework search paths, similar to GCC/Clang
on Darwin.
Key changes include:
DUI::SearchPath
withPathKind {Include, Framework, SystemFramework}
.DUI::searchPaths
is non-empty, it is used verbatim, otherwiseincludePaths
are mirrored as Include paths for backward compatibility.openHeader()
now consults typed paths and rewrites<Pkg/Hdr.h>
toPkg.framework/{Headers,PrivateHeaders}/Hdr.h
only when a package prefix exists.toAppleFrameworkRelatives()
implemented to return prioritized candidates (Headers first, then PrivateHeaders).-F<dir>
and-iframework<dir>
, in addition to-I
.PathKind::Framework
for framework layout.These changes bring simplecpp closer to GCC/Clang behavior on macOS and enable robust resolution of framework headers like
Foundation/Foundation.h
.Supersedes the following pull request: