rjmccall added inline comments. ================ Comment at: include/clang/AST/Type.h:1084 @@ +1083,3 @@ + /// Strip typedefs and atomic from the given type. + QualType getDesugaredAtomicValueType(const ASTContext &ctx) const; + ---------------- Please name this getAtomicUnqualifiedType() and have it promise to remove all qualifiers including _Atomic. It should not promise to remove type sugar.
================ Comment at: lib/AST/Type.cpp:1282 @@ +1281,3 @@ + return *this; +} + ---------------- This should just be: if (auto AT = getAs<AtomicType>()) { return AT->getValueType().getUnqualifiedType(); } else { return getUnqualifiedType(); } ================ Comment at: lib/CodeGen/CGObjC.cpp:901 @@ -903,1 +900,3 @@ + uint64_t IVarSize = getContext().toBits(strategy.getIvarSize()); + llvm::Type *bitcastType = llvm::Type::getIntNTy(getLLVMContext(), IVarSize); bitcastType = bitcastType->getPointerTo(); // addrspace 0 okay ---------------- The code around here uses lowercase identifiers, please be consistent. ================ Comment at: lib/CodeGen/CGObjC.cpp:1023 @@ -1014,1 +1022,3 @@ + value, + ConvertType(propType.getDesugaredAtomicValueType(getContext()))); value = Builder.CreateBitCast( ---------------- I don't understand the purpose of doing two bitcasts here, so let's just drop this first one. ================ Comment at: lib/Sema/SemaDeclObjC.cpp:4319 @@ +4318,3 @@ + Context, MethodLoc, EndLoc, Sel, + resultDeclType.getDesugaredAtomicValueType(Context), + ReturnTInfo, CurContext, MethodType == tok::minus, isVariadic, ---------------- Hmm. I'm not sure you should be doing this here; or at least, if we're going to do it, we should probably be doing it consistently in CheckFunctionReturnType. ================ Comment at: lib/Sema/SemaDeclObjC.cpp:4361 @@ -4361,1 +4360,3 @@ + ObjCMethod, StartLoc, ArgInfo[i].NameLoc, ArgInfo[i].Name, + ArgType.getDesugaredAtomicValueType(Context), DI, SC_None); ---------------- Same thing. If the user wants to explicitly declare a parameter _Atomic, they can do so. ================ Comment at: lib/Sema/SemaObjCProperty.cpp:1497 @@ -1496,2 +1496,3 @@ QualType PropertyIvarType = property->getType().getNonReferenceType(); + PropertyIvarType = PropertyIvarType.getDesugaredAtomicValueType(Context); bool compat = Context.hasSameType(PropertyIvarType, GetterType); ---------------- This is incorrect. The property ivar type should definitely remain _Atomic if the property was declared that way. ================ Comment at: lib/Sema/SemaObjCProperty.cpp:2210 @@ -2209,1 +2209,3 @@ + QualType resultTy = + property->getType().getDesugaredAtomicValueType(Context); if (property->getPropertyAttributes() & ---------------- Please pull this above the comment and give it its own comment, something like: // The getter returns the declared property type with all qualifiers removed. ================ Comment at: lib/Sema/SemaObjCProperty.cpp:2282 @@ -2279,2 +2281,3 @@ QualType paramTy = property->getType().getUnqualifiedType(); + paramTy = paramTy.getDesugaredAtomicValueType(Context); if (property->getPropertyAttributes() & ---------------- Same thing: please pull this above and give it its own comment. http://reviews.llvm.org/D20407 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits