cgen: stop treating every .m as an Objective-C import#26794
Conversation
cgen was using a plain `contains('.m')` check when deciding whether a hash directive should be deferred until after V type definitions. That worked for real Objective-C includes, but it also caught unrelated cases like a `#insert` under a `@VMODROOT` path that happened to include a `.m` directory, or a `#define` value containing `.m`.
When that happened, the directive was moved out of the normal include section and never emitted.
Tighten the check so only `#include` and `#preinclude` directives whose target file actually ends in `.m` get the Objective-C treatment, and add regression tests for both the `#insert` and `#define` cases.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c75c5e2890
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if target.len >= 2 && ((target.starts_with('"') && target.ends_with('"')) | ||
| || (target.starts_with('<') && target.ends_with('>'))) { | ||
| target = target[1..target.len - 1] |
There was a problem hiding this comment.
The new is_objc_hash_target logic only strips quotes when the target both starts and ends with "/<...>, so #include "foo.m" /* note */ is no longer recognized as an Objective-C import. In V parsing, hash directives keep /* ... */ text in node.main (only // is stripped), so this case is reachable and regresses from the previous contains('.m') behavior. When this happens, the .m include is emitted in the normal include section instead of definitions, which can break builds if that imported Objective-C file relies on V type definitions being available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
#include "foo.m" /* note */ is rejected in the checker before cgen runs, so it does not reach is_objc_hash_target. The checker only strips // comments and still requires the target to end in " or >, which makes that form invalid today. So this patch does not regress a reachable Objective-C import case. Supporting trailing /* ... */ after hash directives would be a separate parser/checker change.
cgen was using a plain `contains('.m')` check when deciding whether a hash directive should be deferred until after V type definitions. That worked for real Objective-C includes, but it also caught unrelated cases like a `#insert` under a `@VMODROOT` path that happened to include a `.m` directory, or a `#define` value containing `.m`.
When that happened, the directive was moved out of the normal include section and never emitted.
Tighten the check so only `#include` and `#preinclude` directives whose target file actually ends in `.m` get the Objective-C treatment, and add regression tests for both the `#insert` and `#define` cases.
2 participants
This fixes a cgen bug where C hash directives could disappear from the generated C output if their text happened to contain
.min the wrong place.The old logic used a plain
contains('.m')check to decide whether a directive should be treated as an Objective-C include and deferred until after V type definitions. That was too broad. It could misclassify things like:#insertwith a path that contains.m#definewith a value that contains.mWhen that happened, the directive was moved out of the normal include flow and never emitted.
This change tightens the check so the Objective-C path only applies to
#includeand#preincludedirectives whose target file actually ends with.m.