-
-
Notifications
You must be signed in to change notification settings - Fork 608
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
ImportC: automatically #include importc.h #14121
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#14121" |
756f42a
to
e423061
Compare
| auto f = FileName.combine(entry, "importc.h"); | ||
| if (FileName.exists(f) == 1) | ||
| { | ||
| importc_h = f; |
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.
Where did you clean this? It is going to leak.
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.
FileName.combine() may or may not allocate memory, so it cannot be free'd.
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 you mean we may leak? What FileName.free() does? It seems bad design if we leak resources.
cannot free result of FileName.combine()
| foreach (entry; (global.path ? (*global.path)[] : null)) | ||
| { | ||
| auto f = FileName.combine(entry, "importc.h"); | ||
| if (FileName.exists(f) == 1) |
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.
I thought this would be redundant, but it turns out exists returns a magic number
| error("cannot find \"importc.h\" along import path"); | ||
| fatal(); |
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.
:-(
|
Why is the burden placed on the callee of preprocess in order to find this header? |
Each preprocessor does it differently.