[PATCH] Use dereferencable attribute in Clang for C++ references

[Hal Finkel]

----- Original Message -----
> From: "Richard Smith" <richard at metafoo.co.uk>
> To: "Aaron Ballman" <aaron at aaronballman.com>
> Cc: reviews+D4450+public+b8c95b36670d6323 at reviews.llvm.org, "Hal Finkel" <hfinkel at anl.gov>, "Chandler Carruth"
> <chandlerc at gmail.com>, "Nick Lewycky" <nlewycky at google.com>, "llvm cfe" <cfe-commits at cs.uiuc.edu>
> 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 <
> aaron at aaronballman.com > wrote:
> 
> 
> 
> 
> On Fri, Jul 11, 2014 at 1:50 AM, hfinkel at anl.gov < hfinkel at anl.gov >
> 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