Snapchat / djinni

A tool for generating cross-language type declarations and interface bindings. Djinni's new home is in the Snapchat org.
Apache License 2.0
173 stars 50 forks source link

Djinni generated c++ not compiling in clang-15 #127

Closed JordanMaples closed 1 year ago

JordanMaples commented 1 year ago

To be as up front as possible: I don't think this is a problem in Djinni. However, I figured I'd post here anyway in case someone else hits this problem or wants to reference it.

I wrote the following and generated c++ with it.

widget = record +c {
    test: optional<list<widget>>;
}

The code that this generates (seen in godbolt link below) builds cleanly in the latest MSVC (19.33), GCC (12.2), as well Clang-14. https://godbolt.org/z/q4o5P87xW

In Clang-15 and my locally compiled Clang-16, they both result in compilation errors with these error messages: error: arithmetic on a pointer to an incomplete type 'Widget' error: static assertion failed due to requirement 'std::__is_complete_or_unbounded(std::__type_identity<Widget>{})': template argument must be a complete class or an unbounded array

This error was specific to Clang-16: error: invalid application of 'sizeof' to an incomplete type 'Widget'

li-feng-sc commented 1 year ago

So it seems std::optional<std::vector<Widget>> now requires the full type definition of Widget to compile. Is it possible to include the definition of Widget first in your compilation unit? In your example, if I move struct Widget{}; to the top then it compiles.

Extended records is a feature that exists in the original code but not used by Snap.

JordanMaples commented 1 year ago

I had a test app that I was experimenting against where that approach works sufficiently. However, for the larger app, reordering the include ordering was infeasible due to the scope of our project. I was able to refactor the type to not require the class extension though, so it's fine. Again, I just wanted to make this post mostly as a heads up for other Djinni consumers who are locked into Clang since I can imagine this will hit others when Apple pulls Clang-15 into a future version of XCode.