-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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 errors while semantic checking with incomplete class member pointer type #12442
Comments
This is explained by this comment in SemaType.cpp: // In the Microsoft ABI, the class is allowed to be an incomplete |
...which was added here: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20100809/033284.html |
Sorry for the noise, attached patch to wrong bug. |
Hi Charles, Can you explain the background behind r111118? |
I guess we could fix it by checking if the CXXRecordDecl is incomplete, and return 3 ptrdiff_t's (the worst-case scenario) if it is. Then we can revert r111118. Also, the getMemberPointerSize() method needs to be fixed to obey the '__*_inheritance' attributes that João added. I'm assuming that's what they're for. I'll be happy to commit a patch for that :). (I'm kinda busy right now, though, so actually writing the patch is up to you guys.) |
Thanks for the explanation. That sounds doable. I can take a look into it. |
I did not test this patch yet, just sharing if someone wants to work on it. |
FTR, $ cat incomplete.cpp Foo ptr; int foo(int value) { $ clang -Xclang -cxx-abi -Xclang microsoft -c incomplete.cpp |
Fixed in r163078. |
Fix got reverted. |
Ping? |
I still use the same fix in my local repo. John changed the comment there to add some clarification that we need to early instantiate templates or something there. I asked for his help on how to do that on the list a couple days ago, but I could not find out how to do it with his tips. Unless I get some example on how to do this, I really can't spend time researching on how to do it for the next weeks/months... The fix works well for my use cases though. I think we should apply it meanwhile even if it can fail with some rare template cases... Better have something broken in rare cases than broken under all common cases. Help me get John to accept it :-) |
Can you please share the rebased version of the patch? [+cc: Alexander who can help with Sema/ changes] |
Sure, I will rebase this as soon as I can (probably later today). |
OK, that's much smaller than r163078 was. What's the corner case covered by the FIXME? Does it look something like this? template struct Bar { int Foo::*memptr; Would that not instantiate Foo under Itanium? |
I'm also not sure this makes sense: unsigned MicrosoftCXXABI::getMemberPointerSize(const MemberPointerType *MPT) const {
This only seems correct for member function pointers. Data pointers seem to require one less field, which looks consistent with the code that handles complete types: if (RD->getNumVBases() > 0) { We should be able to query is Pointee is a function or not at this point, right? If so, we should unify the logic for complete and incomplete types. |
You're right, for some reason I completely forgot about data member pointers when writing this code. Your template example makes sense, it's similar to what I was thinking, but the best thing would be to ping John. I agree, it should be possible to unify the code to handle both cases. Can you do that? It would be nice to add some test cases for incomplete types with the inheritance attributes too. |
Sounds good. For tests I started writing some by adding -std=c++11 and using: Although, now I'm not sure that data member pointers are really only size 2 for virtual inheritance. :( I seem to have a counterexample, but I'll have to investigate further. |
Should be fixed as of r178283. |
mentioned in issue llvm/llvm-bugzilla-archive#13707 |
Extended Description
$ cat test.cc
class A {
public:
typedef int (A::*Foo)(int);
};
hummer:src thakis$ ~/src/llvm-svn/Release+Asserts/bin/clang -c test.cc -Xclang -cxx-abi -Xclang microsoft
test.cc:3:19: error: incomplete type 'A' where a complete type is required
typedef int (A::*Foo)(int);
^
test.cc:1:7: note: definition of 'A' is not complete until the closing '}'
class A {
^
1 error generated.
Works fine without -cxx-abi.
The text was updated successfully, but these errors were encountered: