On Dec 8, 2010, at 3:10 AM, Francois Pichet wrote:
> This patch remove the TypesCompatibleExprClass AST node and merge it
> into BinaryTypeTraitExpr.
> A lot of - sign there.
>
> The things to watch out are :
>
> return Owned(new (Context) BinaryTypeTraitExpr(KWLoc, BTT, LhsTSInfo,
> RhsTSInfo, Value, RParen,
> - Context.BoolTy));
> + (BTT == BTT_TypeCompatible)
> ?
> + Context.IntTy :
> Context.BoolTy));
>
> BTT_TypeCompatible must have an int type otherwise some lit tests fail.
It's fine for different type traits to have different return types, but please
use a switch() that lists all of the cases. That when, when a new type trait
gets added, we'll be forced to update the switch appropriately.
FWIW, BTT_TypeCompatible has 'int' type because it's a GNU C extension.
>
> Also this:
> Value *VisitBinaryTypeTraitExpr(const BinaryTypeTraitExpr *E) {
> - return llvm::ConstantInt::get(Builder.getInt1Ty(), E->getValue());
> + return llvm::ConstantInt::get(ConvertType(E->getType()), E->getValue());
> }
Yes, that makes sense.
Everything else looks good, thanks!
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
