[PATCH] D34120: [IR] Stop trying to delete other signatures of User::operator new when we override one signature in a class derived from User

[David Blaikie via llvm-commits]

On Mon, Jun 12, 2017 at 3:19 PM Craig Topper via Phabricator <
reviews at reviews.llvm.org> wrote:

> craig.topper created this revision.
>
> User has 3 signatures for operator new today. They take a single size, a
> size and a number of users, and a size, number of users, and descriptor
> size.
>
> Historically there used to only be one signature that took size and a
> number of uses. Long ago derived classes implemented their own versions
> that took just a size and would call the size and use count version. Then
> they left an unimplemented signature for the size and use count signature
> from User. As we moved to C++11 this unimplemented signature because =
> delete.
>
> Since then operator new has picked up two new signatures for operator new.
> But when the 3 argument version was added it was never added to the delete
> list in all of the derived classes where the 2 argument version is deleted.
> This makes things inconsistent.
>
> I believe once one version of operator new is created in a derived class
> name hiding will take care of making all of the base class signatures
> unavailable. So I don't think the deleted lines are needed at all.
>

I know LLVM doesn't have any "no compile" tests (that demonstrate certain
things fail to compile), but have you tried this manually/locally & could
you share those results in this review, so it's clear this
workaround/cliche isn't necessary?

(show the error message before & after this change)

If that's clearly demonstrated, then I'd say go ahead with this.


>
> This patch removes all of the deletes in cases where there is an override
> or there is already a delete of another signature (that should trigger name
> hiding too).
>
>
> https://reviews.llvm.org/D34120
>
> Files:
>   include/llvm/Analysis/MemorySSA.h
>   include/llvm/IR/Constants.h
>   include/llvm/IR/GlobalVariable.h
>   include/llvm/IR/InstrTypes.h
>   include/llvm/IR/Instructions.h
>   include/llvm/IR/Operator.h
>   lib/IR/ConstantsContext.h
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170612/142d9729/attachment-0001.html>