__is_empty type trait patch

Hello,

I'm a new coder hoping to help out with the C++ implementation on clang.
After fiddling around with several things I discovered were all being
worked on already, someone recommended I implement the __is_empty type
trait, so that is what I did. I was told to send a message here now;
I've attached a patch.

A few notes about this:

I put tests in test/SemaCXX/type-traits.cpp. There are a bunch of tests
I added to __is_pod and __is_empty, mainly for testing purposes. I was
going to remove them, but then I figured there's no sense removing tests
- more is better.

I also encountered, while doing checks for zero-width bit-fields, an
error "bit-field i has zero width". I added the word "named" to the
diagnostic so that it's clear what the error really is. This change is
unrelated but is minor so I included it anyways.

Sean Hunt

__is_empty.patch (14.3 KB)

Hello,

I'm a new coder hoping to help out with the C++ implementation on clang.
After fiddling around with several things I discovered were all being
worked on already, someone recommended I implement the __is_empty type
trait, so that is what I did. I was told to send a message here now;
I've attached a patch.

Cool.

A few notes about this:

I put tests in test/SemaCXX/type-traits.cpp. There are a bunch of tests
I added to __is_pod and __is_empty, mainly for testing purposes. I was
going to remove them, but then I figured there's no sense removing tests
- more is better.

Yeah, having a bunch of tests is good. It's generally not a good idea
to put in commented-out tests, though; if it gives an error, use an
expected-error marking, and put a comment that the error isn't
correct. (If it crashes, of course, it's better to leave it commented
out.)

I also encountered, while doing checks for zero-width bit-fields, an
error "bit-field i has zero width". I added the word "named" to the
diagnostic so that it's clear what the error really is. This change is
unrelated but is minor so I included it anyways.

I committed this separately.

bool Sema::VerifyBitField(SourceLocation FieldLoc, IdentifierInfo *FieldName,
- QualType FieldTy, const Expr *BitWidth) {

Eli Friedman wrote:

Yeah, having a bunch of tests is good. It's generally not a good idea
to put in commented-out tests, though; if it gives an error, use an
expected-error marking, and put a comment that the error isn't
correct. (If it crashes, of course, it's better to leave it commented
out.)

Ah. Virtual bases are unsupported, so I figured commenting out the tests
until they are was the best choice of action.

This looks suspicious; is it really supposed to default to true? (At
first glance, it looks like it will misclassify a struct with only
bitfields.)

Otherwise, looks fine.

-Eli

Yes, because if the width expression is dependent, it shouldn't cause
the class to be marked as non-empty at that point in the code (every
other exit point should be a false return, I think). Come to think of
it, I think I forgot to add a check when instantiating a template.

I'll go off to check that.

Sean Hunt

Okay; that makes sense; the reason it looks suspicious is that there
isn't any path where it subsequently gets set to false.

-Eli

2 aug 2009 kl. 19.45 skrev Sean Hunt:

Hello,

I'm a new coder hoping to help out with the C++ implementation on clang.
After fiddling around with several things I discovered were all being
worked on already, someone recommended I implement the __is_empty type
trait, so that is what I did. I was told to send a message here now;
I've attached a patch.

Nice, welcome! :slight_smile:

+ /// Empty - true when this class is empty for traits purposes, i.e. has no
+ /// data members other than 0-width bit-fields, has no virtual function/base,
+ /// and doesn't inherit from a non-empty class. Doesn't take union-ness into
+ /// account.
+ bool Empty : 1;
+

You don't have to store this - the ASTRecordLayout already stores the dataSize so you can just check to see if it's 0.

Anders

That doesn't actually work; see http://llvm.org/bugs/show_bug.cgi?id=4673 .

-Eli