atilaneves / dpp

Directly include C headers in D source code
Boost Software License 1.0
229 stars 31 forks source link

core.stdc definition conflicts with ldc2 #286

Open aminya opened 3 years ago

aminya commented 3 years ago

When I use ldc2 to build a C library, I get these kinds of errors if my d file uses std.stdio

dmd works fine without any issues!

> d++ --compiler=ldc2 file.dpp file.lib entry.d

C:\tools\ldc2-1.24.0-windows-x64\bin\..\import\core\stdc\stdio.d(1317): Error: Function type does not match previously declared function with the same mangled name: `fwrite`
C:\tools\ldc2-1.24.0-windows-x64\bin\..\import\core\stdc\stdio.d(1317):        Previous IR type: i64 (i8*, i64, i64, %file._iobuf*)
C:\tools\ldc2-1.24.0-windows-x64\bin\..\import\core\stdc\stdio.d(1317):        New IR type:      i64 (i8*, i64, i64, %core.stdc.stdio._iobuf*)

This is because I have writeln("hello"); in the main function of entry.d which imports file.

I noticed that dpp generates these conflicting definitions.

    alias FILE = _iobuf;
    struct _iobuf
    {
        void* _Placeholder;
    }
    ulong fwrite(const(void)*, ulong, ulong, _iobuf*) @nogc nothrow;

The issue is the same on Ubuntu or Windows.

aminya commented 3 years ago

I found the reason for this. Adding --ignore-system-paths fixes the issue on Windows. The issue seems to be some runtime mismatch when using ldc (it uses CRuntime_DigitalMars by default?).

One of the windows headers defines _iobuf as the following:

    alias FILE = _iobuf;
    struct _iobuf
    {
        void* _Placeholder;
    }

But by adding --ignore-system-paths, this is replaced with an empty declaration:

struct _iobuf;

The definition of _iobuf for ldc in CRuntime_DigitalMars is here: https://github.com/ldc-developers/druntime/blob/c9f4bde0eea78070773835a3d559d5216a48ac64/src/core/stdc/stdio.d#L371-L381

Which is different from what dmd defines for CRuntime_Microsoft: https://github.com/dlang/druntime/blob/e85d9ee0d789966dd059951279ddaffac869f6ba/src/core/stdc/stdio.d#L392-L398

atilaneves commented 3 years ago

Without the .dpp file it's impossible for me to look at this.

aminya commented 3 years ago

I will try to create a minimal working example. Although do above workaround works for other functions, but when I have to actually pass a file.getFP from D to C, it doesn't work. So, it is worth investigating the issue more.

aminya commented 3 years ago

Here are a couple of files that reproduce the issue.

dub run --build=release --quiet --yes dpp -- --preprocess-only ./test.dpp  --keep-pre-cpp-files

test_dpp.zip

dub run --build=release --quiet --yes dpp -- --preprocess-only ./test.dpp  --keep-pre-cpp-files --ignore-system-paths

test_dpp_ignore_system_paths.zip

dub run --build=release --quiet --yes dpp -- --preprocess-only ./test.dpp  --keep-pre-cpp-files --no-sys-headers

test_dpp_no_sys_headers.zip

(non-sys-headers don't seem to have any effects.)

dub run --build=release --quiet --yes dpp -- --preprocess-only ./test.dpp  --keep-pre-cpp-files --prebuilt-header stdio.h=core.stdc.stdio --prebuilt-header stdlib.h=core.stdc.stdlib --prebuilt-header stdint.h=core.stdc.stdint 

test_dpp_specify_core-stdc.zip

dub run --build=release --quiet --yes dpp -- --preprocess-only ./test.dpp  --keep-pre-cpp-files --prebuilt-header stdio.h=core.stdc.stdio --prebuilt-header stdlib.h=core.stdc.stdlib --prebuilt-header stdint.h=core.stdc.stdint --ignore-path stdio.h --ignore-path stdlib.h --ignore-path stdint.h

test_dpp_specify_and_ignore_core-stdc.zip

(Dpp doesn't seem to actually ignore these headers.)

dub run --build=release --quiet --yes dpp -- --preprocess-only ./test.dpp  --keep-pre-cpp-files --prebuilt-header stdio.h=core.stdc.stdio --prebuilt-header stdlib.h=core.stdc.stdlib --prebuilt-header stdint.h=core.stdc.stdint --ignore-path stdio.h --ignore-path stdlib.h --ignore-path stdint.h  --ignore-system-paths

test_dpp_specify_core-std_ignore_sytem_paths.zip

(this one could be made to work if struct _iobuf; would have imported from core.stdc.stdio).

aminya commented 3 years ago

One short term solution to this is to extern the unknown definitions instead of defining them like struct _iobuf;

In the long term, there should be a way to specify prebuilt headers for the standard libraries (C and C++). For example, D has core.stdc headers for most of the C headers, but Dpp doesn't use those, which results in these bugs.