atilaneves / dpp

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

class value type inheriting from two other aggregates is broken #117

Open Laeeth opened 5 years ago

Laeeth commented 5 years ago

obviously we don't have multiple inheritance in d. but most important thing is to get the layout right. currently we have:

class Currency: Convention<CurrencyId>, public relational_operators<Currency>
{}

translated as

struct Currency
{
  private:
    Convention!CurrencyId __base;
    alias __base this;
    relational_operators!Currency __base;
     alias __base this;
}

which is not going to get us very far.

so I don't know how multiple inheritance layout works. but if you just stick them in order then number them base1, base2 etc and worry about no multiple alias this later.

atilaneves commented 5 years ago

Multiple inheritance hasn't been implemented yet. The idea is to have a heuristic to decide to translate a C++ struct/class to either a D struct or D class. Currently it only does structs, and it's possible that even for multiple inheritance the right decision is still a struct. The layout can be worked out then, but there's no multiple alias this yet in D.

Laeeth commented 5 years ago

Yes, but what I mean is that presuming you have the data layout right then it wouldn't matter for some purposes if there were no such thing as alias this. Being able to represent the record layout correctly of c++ library is pretty valuable.

Laeeth commented 5 years ago

I know there's no multiple alias this in D - that's what I said. If you get records right then can worry about the absence of multiple alias this later. There's a decent chance that before dpp is complete we do have multiple alias this in the compiler. If not, then if a class is used as a value type in another class then it certainly isn't going to work if you don't have record layout right...

atilaneves commented 5 years ago

I've had an idea about private members: Don't try to translate them at all. Instead, grab their size and alignment from libclang and declare a static array of that many bytes instead. In most real-world C++ code, fields are all private anyway.

Laeeth commented 5 years ago

That's what my PR does

Laeeth commented 5 years ago

Except it doesn't pick by private members

Laeeth commented 5 years ago

You want to do this for the combination (I won't use the word union!) of private types, blacklisted types, untranslatable types.

Laeeth commented 5 years ago

I just set the size which will be wrong wrt alignment

Laeeth commented 5 years ago

For public members blacklisted or untranslatable you probably rather make it an Opaque(string name, size_t size)

Laeeth commented 5 years ago

If you do the above, fix operators, fix the fact the forward declaration problem, fix multiple inheritance (just number them) then I think quite a lot of c++ headers will be usable

Laeeth commented 5 years ago

Note that once dpp can compile other clang headers then many more things become easier.

atilaneves commented 5 years ago

I merged your PR into a branch of mine so that I can take it from there.