Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang-cl: complains 'inheritance model does not match definition' #20361

Closed
jrmuizel opened this issue Jun 10, 2014 · 9 comments
Closed

clang-cl: complains 'inheritance model does not match definition' #20361

jrmuizel opened this issue Jun 10, 2014 · 9 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla clang Clang issues not falling into any other category

Comments

@jrmuizel
Copy link

Bugzilla Link 19987
Resolution FIXED
Resolved on Jun 23, 2015 11:19
Version unspecified
OS All
CC @majnemer,@ehsan,@rnk

Extended Description

Clang fails to compile the following code that succeeds with cl.exe

class A {};
template class B {
void (T::*mCallOnResume)();
};
namespace q {
class C : A, B {
};
};

@jrmuizel
Copy link
Author

assigned to @majnemer

@jrmuizel
Copy link
Author

Here's a slightly saner reduction:

class A {};
template class B {
void (T::*mCallOnResume)();
};
class C : A, B {
};

C p;

@rnk
Copy link
Collaborator

rnk commented Jun 10, 2014

As a workaround, you can do this:

class A {};
template class B {
void (T::*mCallOnResume)();
};
class __multiple_inheritance C; // workaround
class C : A, B {
};

Our diagnostic is bad, but here's what's happening:

  • We parse C's base specifiers
  • Instantiate B
  • RequireCompleteType on 'void (C::*)()'
  • RequireCompleteType on C, lock in the inheritance model, probably as single inheritance, because we haven't set C's base specifiers yet
  • Finish C's definition, realize that we assumed C was using single inheritance when it really used multiple inheritance.

@ehsan
Copy link
Contributor

ehsan commented Jun 10, 2014

Thanks a lot for the analysis and workaround, Reid! I'll try to work with Jeff tomorrow to see how we can apply this workaround to our code. I have to admit that I don't entirely understand the implications of these MSVC keywords (I just learned about their existence yesterday.) I assume the workaround won't change how the code will behave when built with cl?

@majnemer
Copy link
Mannequin

majnemer mannequin commented Jun 13, 2014

marginally reduced:
template
class A {
int T::*x;
};
class B : virtual A {};

@majnemer
Copy link
Mannequin

majnemer mannequin commented Jun 13, 2014

Fixed in r210886.

@rnk
Copy link
Collaborator

rnk commented Jun 18, 2014

I should point out that while we have fixed this in clang, note that sizeof(mCallOnResume) will probably be 16 (1 pointer and 3 ints!) on x86 with both MSVC and Clang. That's probably larger than you wanted. The __multiple_inheritance workaround that I gave should make it just 8 (1 pointer and 1 int).

@llvmbot
Copy link
Collaborator

llvmbot commented Jun 23, 2015

It seems the fix is not 100% correct.
We are using clang through libTooling, -v gives:
clang version 3.6.0 (branches/release_36 230502) (llvm/branches/release_36 230501).

code (simplified) :
class A : public B<wchar_t>
{
public:
// ...
typedef bool (A::*C)(const D<wchar_t>&, E&, int, const F&);
// ...
static C m[];
// ...
};

clang output:
In file included from ...
...
a.h:95:1: error: inheritance model does not match definition
class A : public B<wchar_t>
^
a.h:95:7: note: A defined here
class A : public B<wchar_t>
^
1 error generated.
Error while processing ....cpp.

@majnemer
Copy link
Mannequin

majnemer mannequin commented Jun 23, 2015

József, I cannot reproduce with what you have posted. Please provide a complete example.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang Clang issues not falling into any other category
Projects
None yet
Development

No branches or pull requests

4 participants