[cfe-commits] r59543 - in /cfe/trunk/lib/CodeGen: CGExpr.cpp CGObjCMac.cpp CGValue.h

[Daniel Dunbar]

Hi Fariborz,

Some minor nitpicks:

On Tue, Nov 18, 2008 at 12:18 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>   } else if (VD && VD->isFileVarDecl()) {
> -    return LValue::MakeAddr(CGM.GetAddrOfGlobalVar(VD),
> -                            E->getType().getCVRQualifiers());
> +    LValue LV = LValue::MakeAddr(CGM.GetAddrOfGlobalVar(VD),
> +                                 E->getType().getCVRQualifiers());
> +    if (VD->getAttr<ObjCGCAttr>())
> +    {
> +      ObjCGCAttr::GCAttrTypes attrType = (VD->getAttr<ObjCGCAttr>())->getType();

This is a little more idiomatic, also note clang tends to have braces
on same line as condition:

if (ObjCGCAttr *A = VD->getAttr<ObjCGCAttr>()) {
     ObjCGCAttr::GCAttrTypes attrType = A->getType();

> +  bool ObjcWeak:1;
> +  bool ObjcStrong:1;

Can this be changed to 'unsigned ObjCType : 2', with an appropriate
enum for None, Weak, Strong? This makes it clear that we have a free
value and that weak and strong are mutually exclusive. This simplifies
the declaration of SetObjCGCAttrs (which I would call SetObjCType to
match the field).

> +  bool isObjcWeak() const { return ObjcWeak; }
> +  bool isObjcStrong() const { return ObjcStrong; }

These should be spelled ObjC for internal consistency.

Thanks,
 - Daniel