----- Original Message ----- > From: "Richard Smith" <[email protected]> > To: "Aaron Ballman" <[email protected]> > Cc: [email protected], "Hal Finkel" > <[email protected]>, "Chandler Carruth" > <[email protected]>, "Nick Lewycky" <[email protected]>, "llvm cfe" > <[email protected]> > Sent: Wednesday, July 16, 2014 5:46:41 PM > Subject: Re: [PATCH] Use dereferencable attribute in Clang for C++ references > > > My comments were the same as Aaron's =) > > > Other than that, LGTM.
Great, thanks! I'll commit with those changes if/when the underlying LLVM patch lands. -Hal > > > On Fri, Jul 11, 2014 at 9:49 AM, Aaron Ballman < > [email protected] > wrote: > > > > > On Fri, Jul 11, 2014 at 1:50 AM, [email protected] < [email protected] > > wrote: > > Fixed up the remaining regression tests (now all pass), and added a > > new test for dereferenceable/nonnull on non-addrspace(0) > > references. > > > > http://reviews.llvm.org/D4450 > > > > Files: > > lib/CodeGen/CGCall.cpp > > test/CXX/except/except.spec/p14-ir.cpp > > test/CodeGenCXX/address-space-ref.cpp > > test/CodeGenCXX/blocks-cxx11.cpp > > test/CodeGenCXX/blocks.cpp > > test/CodeGenCXX/catch-undef-behavior.cpp > > test/CodeGenCXX/conditional-gnu-ext.cpp > > test/CodeGenCXX/const-init-cxx11.cpp > > test/CodeGenCXX/constructor-direct-call.cpp > > test/CodeGenCXX/constructor-init.cpp > > test/CodeGenCXX/convert-to-fptr.cpp > > test/CodeGenCXX/copy-assign-synthesis-1.cpp > > test/CodeGenCXX/copy-constructor-elim-2.cpp > > test/CodeGenCXX/copy-constructor-synthesis-2.cpp > > test/CodeGenCXX/copy-constructor-synthesis.cpp > > test/CodeGenCXX/cxx11-initializer-aggregate.cpp > > test/CodeGenCXX/cxx11-thread-local-reference.cpp > > test/CodeGenCXX/decl-ref-init.cpp > > test/CodeGenCXX/default-arg-temps.cpp > > test/CodeGenCXX/derived-to-base-conv.cpp > > test/CodeGenCXX/derived-to-virtual-base-class-calls-final.cpp > > test/CodeGenCXX/dllexport-members.cpp > > test/CodeGenCXX/dllexport.cpp > > test/CodeGenCXX/dllimport-members.cpp > > test/CodeGenCXX/dllimport.cpp > > test/CodeGenCXX/eh.cpp > > test/CodeGenCXX/empty-nontrivially-copyable.cpp > > test/CodeGenCXX/exceptions.cpp > > test/CodeGenCXX/fastcall.cpp > > test/CodeGenCXX/goto.cpp > > test/CodeGenCXX/implicit-copy-assign-operator.cpp > > test/CodeGenCXX/implicit-copy-constructor.cpp > > test/CodeGenCXX/mangle-lambdas.cpp > > test/CodeGenCXX/mangle.cpp > > test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp > > test/CodeGenCXX/microsoft-abi-static-initializers.cpp > > test/CodeGenCXX/nrvo.cpp > > test/CodeGenCXX/pod-member-memcpys.cpp > > test/CodeGenCXX/pointers-to-data-members.cpp > > test/CodeGenCXX/reference-cast.cpp > > test/CodeGenCXX/rvalue-references.cpp > > test/CodeGenCXX/temporaries.cpp > > test/CodeGenCXX/throw-expressions.cpp > > test/CodeGenCXX/volatile.cpp > > test/CodeGenObjC/ return-objc-object.mm > > test/CodeGenObjCXX/ arc-blocks.mm > > test/CodeGenObjCXX/ arc-move.mm > > test/CodeGenObjCXX/ arc-special-member-functions.mm > > test/CodeGenObjCXX/ implicit-copy-assign-operator.mm > > test/CodeGenObjCXX/ implicit-copy-constructor.mm > > test/CodeGenObjCXX/ lvalue-reference-getter.mm > > test/CodeGenObjCXX/ message-reference.mm > > test/CodeGenObjCXX/ property-dot-reference.mm > > test/CodeGenObjCXX/ property-lvalue-capture.mm > > test/CodeGenObjCXX/ property-object-reference-2.mm > > test/CodeGenObjCXX/ property-objects.mm > > test/CodeGenObjCXX/ property-reference.mm > > test/Modules/ templates.mm > > I have no real comments on whether this is the correct approach or > not, but I do have a small style nit: > > > > Index: lib/CodeGen/CGCall.cpp > > =================================================================== > > --- lib/CodeGen/CGCall.cpp > > +++ lib/CodeGen/CGCall.cpp > > @@ -1200,8 +1200,14 @@ > > > llvm_unreachable("Invalid ABI kind for return argument"); > > } > > > > - if (RetTy->isReferenceType()) > > - RetAttrs.addAttribute(llvm::Attribute::NonNull); > > > + if (RetTy->isReferenceType()) { > > + QualType PTy = RetTy->getAs<ReferenceType>()->getPointeeType(); > > if (const auto *RefTy = RetTy->getAs<ReferenceType>()) { > QualType Pty = RefTy->getPointeeType(); > ... > > > + if (!PTy->isIncompleteType() && PTy->isConstantSizeType()) > > + > > RetAttrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy) > > + .getQuantity()); > > + else if (getContext().getTargetAddressSpace(PTy) == 0) > > + RetAttrs.addAttribute(llvm::Attribute::NonNull); > > + } > > > > if (RetAttrs.hasAttributes()) > > PAL.push_back(llvm:: > > @@ -1291,8 +1297,14 @@ > > } > > } > > > > - if (ParamType->isReferenceType()) > > - Attrs.addAttribute(llvm::Attribute::NonNull); > > > + if (ParamType->isReferenceType()) { > > + QualType PTy = > > ParamType->getAs<ReferenceType>()->getPointeeType(); > > Same here as above. > > > + if (!PTy->isIncompleteType() && PTy->isConstantSizeType()) > > + Attrs.addDereferenceableAttr(getContext().getTypeSizeInChars(PTy) > > + .getQuantity()); > > + else if (getContext().getTargetAddressSpace(PTy) == 0) > > > > + Attrs.addAttribute(llvm::Attribute::NonNull); > > + } > > ~Aaron > > -- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
