Support one-level inheritance cases

[lixin-wei]

Resolves #82

Please squash merge this PR.

[lixin-wei]

@liuzicheng1987 Hi I'm done. Would you please have a look when you have time?

[liuzicheng1987]

@lixin-wei, wow, that was fast.

I will take a closer look later today, but at first glance it seems good.

Thank you for your contribution!

[lixin-wei]

@schaumb Thanks for the review. All comments fixed/replied. Would you pls have a look again?

[liuzicheng1987]

@lixin-wei , great work!

I have two very minor requests:

1. Could you place your test in a separate test and call it "test_inheritance2"? The reason I think this is a good idea is that there is already "test_inheritance" which basically tests the flip case of what you have implemented: https://github.com/lixin-wei/reflect-cpp/blob/derived_srturct/tests/json/test_inheritance.cpp I also think every test should only test one thing. I realize that your test is a compile-time test and I think that is a very good thing. Just std::cout OK at the end for consistency.

2. The word "constructible" is spelled with an i, not an a...I realize that that doesn't make a lot of sense, but English spelling is notoriously inconsistent. So could you rename your function constructable_no_brace_elision?

[lixin-wei]

@liuzicheng1987 All done, would you please have a look again? thanks for taking the time to review!
Note that please squash merge my PR because there are a lot of noisy commit like 'fix CI', 'fix coments'.

[schaumb]

This is only true with this implementation, but not necessarily.

You can check which base class is 'empty' and which is not with another ubiq:

template <class Derived>
struct any_empty_base {
  any_empty_base(std::size_t);
  template <class Base>
    requires(
        std::is_empty_v<std::remove_cvref_t<Base>> && // new check
        std::is_base_of_v<std::remove_cvref_t<Base>,
                          std::remove_cv_t<Derived>> &&
        !std::is_same_v<std::remove_cvref_t<Base>, std::remove_cv_t<Derived>>)
  constexpr operator Base&() const noexcept;
};

T(any_empty_base(Bx...), any_base(N), any_empty_base(Ax...)).
If no one is creatable (N <- [1..base_args]) then it is not a structure bindable class (static_assert).

[schaumb]

This should be implemented with a recursion somehow because there are classes with multiple inheritances that can be used with structure bind.

struct B1 {};
struct B2 {};
struct B3 {};
struct M { int a; int b; };
struct D1 : B1, M, B3 {};
struct D2 : B2, D1, B3 {};

// ...
// D2 d;
// rfl::to_view(d) // recognized 3, but only 2 member

[liuzicheng1987]

@lixin-wei , as far as squash merges are concerned, I am usually a big proponent.

My concern is authorship...when people contribute they like to see themselves as contributors, but when I squash + merge, then I become the author (at least in some cases). So I would in effect be claiming authorship for something that I didn't write. I think that for open source projects, this is a problem.

But I will squash + merge this one.

[lixin-wei]

but when I squash + merge, then I become the author (at least in some cases).

@liuzicheng1987 Really? I don't think so. Github does handle this when you click 'squash merge'. The author won't be you.

For example, this commit is a squash merge from this PR. The one who squash merged the PR is not the author but I am.

Besides, squash merge commit is very easy to find the corresponding PR from the commit message. While it's very hard for non-squashed merging. I highly recomend setting this as default.

[liuzicheng1987]

@lixin-wei , I have had this issue under some circumstances. It is possible that this only happens when I have to resolve merge conflicts.

But I am certainly not the only one who has had this problem:

https://stackoverflow.com/questions/39700184/git-squash-commits-while-retaining-authors-info

Again, the issue here might be merge conflicts...

I will certainly research this issue, because I also prefer squash + merge.

[liuzicheng1987]

@lixin-wei , do you want to implement @schaumb latest input? I think the multiple inheritance edge case is a valid point.

[lixin-wei]

@liuzicheng1987 Yes I'm working on it, there are so many corner cases... 😭 I'll ping you when I'm finished.

[lixin-wei]

I have had this issue under some circumstances. It is possible that this only happens when I have to resolve merge conflicts.

@liuzicheng1987 It's author's responsibility to resolve the conflicts. You shouldn't do this.

If you do help the author to resolve the conflicts. Then you do 'contributed' to this PR. There is no problem to put you into the author list. Github will show all persons contributed to the PR in the squash merge commit, like the following image:

[liuzicheng1987]

@lixin-wei , I will just give it a try. I will just make squash + merge the default. If there are any issues along the lines I have outlined, I can always change it back.

[lixin-wei]

Hi @liuzicheng1987 I solved the one-level multi inheritance with the any_empty_base mentioned above.
Thanks @schaumb !

But the multi-level inheritance case(as the following code shows) is very tricky and I don't have a very good solution😂 Do you guys have any idea? Can we merge this first and open another PR to tackle this hard case?

struct B1 {};
struct B2 {};
struct B3 {};
struct M { int a; int b; };
struct D1 : B1, M, B3 {};
struct D2 : B2, D1, B3 {};

[liuzicheng1987]

@lixin-wei , I think so, too...I mean this is a bit of an edge case anyway. The possible use cases for a multiple inheritance structure like this are fairly limited anyway...

[liuzicheng1987]

@lixin-wei , seems you were right. I squashed + merged this and you are now the author of the squashed merge in the git history.

[lixin-wei]

@liuzicheng1987 I also remcomend you to install this plugin, which will automatically set the commit message from PR's description. The default squash commit message of GitHub is very ugly.

[liuzicheng1987]

@lixin-wei , cool, I didn't know that one!

[schaumb]

I created a multi-level CountFieldsHelper here:
https://godbolt.org/z/bqsq5Wv7s

[lixin-wei]

@schaumb That's awesome. I've learned a lot from your code.

[liuzicheng1987]

@lixin-wei , if you want to open another PR, feel free to do so. I would love to have this.