Thread overview
Continued: Declarations of the same C struct conflict when imported
Sep 05, 2019
György Andrasek
Sep 06, 2019
Kagamin
Sep 06, 2019
Timon Gehr
Sep 06, 2019
Timon Gehr
Sep 06, 2019
Timon Gehr
Sep 06, 2019
György Andrasek
Sep 07, 2019
Jacob Carlborg
September 05, 2019
Hi,

I accidentally ran dstep twice and ended up with the following interaction:

https://github.com/ldc-developers/ldc/issues/3155

---

ldc 1.17.0, Android/Termux.

libarchive comes in two public header files: `archive.h` and `archive_entry.h`.

Both headers declare `struct archive;` and `struct archive_entry;`, faithfully
reproduced by dstep. Trying to import both yields a conflict immediately:

```
[  6%] Building D object CMakeFiles/wng.dir/src/wngbase/libarchive.o
/data/data/com.termux/files/home/weidu-ng/src/wngbase/libarchive.d(27): Error: `archive_h.archive` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_h.d(122) conflicts with `archive_entry_h.archive` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_entry_h.d(92)
/data/data/com.termux/files/home/weidu-ng/src/wngbase/libarchive.d(28): Error: `archive_h.archive_entry` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_h.d(123) conflicts with `archive_entry_h.archive_entry` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_entry_h.d(93)
```

Annoying, but alias can deal with it. The problem comes in actually using them. Annotated for clarity:

```D
int archive_read_next_header(archive_h.archive*, archive_h.archive_entry**);

const(char)* archive_entry_pathname_utf8(archive_entry_h.archive_entry*);
```

(I'm guessing this is the correct place to report this since this is a notable improvement from ldc 1.13, which didn't even understand pointer-to-opaque.)

---

They told me to go away. :)

I'm not trying to redesign the language, it simply didn't occur to me that this won't work or isn't a trivial oversight. (Android is dark and full of terrors.)

I can't imagine how it makes the language better to disallow cross-module C calls like this, given that the least ugly workaround is `void *`.

P.S. @kinke went through the trouble of upgrading ldc on Termux while I was busy installing cargo. Android is now ahead of Ubuntu. Please give him a medal or something.
September 06, 2019
The solution for ambiguity is disambiguation, not void*.
September 06, 2019
On 06.09.19 10:31, Kagamin wrote:
> The solution for ambiguity is disambiguation, not void*.

There is no ambiguity. The error is obviously silly. It makes no sense to force the user to explicitly disambiguate between two equivalent options.
September 06, 2019
On 06.09.19 01:19, György Andrasek wrote:
> Annoying, but alias can deal with it. The problem comes in actually using them. Annotated for clarity:
> 
> ```D
> int archive_read_next_header(archive_h.archive*, archive_h.archive_entry**);
> 
> const(char)* archive_entry_pathname_utf8(archive_entry_h.archive_entry*);
> ```
> 
> (I'm guessing this is the correct place to report this since this is a notable improvement from ldc 1.13, which didn't even understand pointer-to-opaque.)
> 

Actually, the correct place is https://issues.dlang.org (issues posted only here are likely to get forgotten because nobody tracks issues discussed on the forums).

> ---
> 
> They told me to go away. :)
> ...

LDC is mostly a compiler backend effort. Updating the DMD frontend will usually update LDC quickly after.

> I'm not trying to redesign the language,

You actually are, but that's not a bad thing.

> it simply didn't occur to me that this won't work or isn't a trivial oversight. (Android is dark and full of terrors.)
> ...

Making this work requires changing the language non-trivially.

> I can't imagine how it makes the language better to disallow cross-module C calls like this, given that the least ugly workaround is `void *`.

It doesn't make the language any better, but the way this is phrased makes little sense. The current behavior requires no additional effort. `extern(C)` just doesn't affect symbol identity on the D side:

---
module a;
import std.stdio;
extern(C) struct S;
template test(alias x){ int test; }
import b;

void main(){
    test!(S)=2;
    test!(b.S)=3;
    writeln(test!S," ",test!(b.S)); // 2 3
}
---

---
module b;
extern(C) struct S;
---

So even though the two declarations of S would refer to the same type on the C side, this is not the case on the D side. The code above is just one example of code that needs to change behavior to support the feature you want.
September 06, 2019
On 06.09.19 01:19, György Andrasek wrote:
> given that the least ugly workaround is `void *`.

Given that you are changing the generated headers anyway, why isn't the least ugly workaround to delete the conflicting declarations in one of them and importing them from the other one?
September 06, 2019
On Friday, 6 September 2019 at 10:28:02 UTC, Timon Gehr wrote:
> On 06.09.19 01:19, György Andrasek wrote:
>> given that the least ugly workaround is `void *`.
>
> Given that you are changing the generated headers anyway, why isn't the least ugly workaround to delete the conflicting declarations in one of them and importing them from the other one?

It might make more sense if you think of `void *` as a swear word.
September 07, 2019
On 2019-09-06 01:19, György Andrasek wrote:
> Hi,
> 
> I accidentally ran dstep twice and ended up with the following interaction:
> 
> https://github.com/ldc-developers/ldc/issues/3155
> 
> ---
> 
> ldc 1.17.0, Android/Termux.
> 
> libarchive comes in two public header files: `archive.h` and `archive_entry.h`.
> 
> Both headers declare `struct archive;` and `struct archive_entry;`, faithfully
> reproduced by dstep. Trying to import both yields a conflict immediately:
> 
> ```
> [  6%] Building D object CMakeFiles/wng.dir/src/wngbase/libarchive.o
> /data/data/com.termux/files/home/weidu-ng/src/wngbase/libarchive.d(27): Error: `archive_h.archive` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_h.d(122) conflicts with `archive_entry_h.archive` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_entry_h.d(92) 
> 
> /data/data/com.termux/files/home/weidu-ng/src/wngbase/libarchive.d(28): Error: `archive_h.archive_entry` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_h.d(123) conflicts with `archive_entry_h.archive_entry` at /data/data/com.termux/files/home/weidu-ng/src/wngbase/binding/archive_entry_h.d(93) 
> 
> ```

It's easier if you can show the code. Is one declaration a forward reference and the other a definition or are both forward references? It's a known limitation of DStep. DStep would need to look at all files and if one is a forward reference replace that with an import. If all declarations are forward references I guess DStep would need to arbitrary pick one as the "main" one and replace the other ones with imports and/or aliases. I know this issue has come up before. I might already have been reported here [1].

[1] https://github.com/jacob-carlborg/dstep/issues

-- 
/Jacob Carlborg