<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 12, 2017 at 3:19 PM Craig Topper via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">craig.topper created this revision.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.<br></blockquote><div><br>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?<br><br>(show the error message before & after this change)<br><br>If that's clearly demonstrated, then I'd say go ahead with this.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
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).<br>
<br>
<br>
<a href="https://reviews.llvm.org/D34120" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34120</a><br>
<br>
Files:<br>
  include/llvm/Analysis/MemorySSA.h<br>
  include/llvm/IR/Constants.h<br>
  include/llvm/IR/GlobalVariable.h<br>
  include/llvm/IR/InstrTypes.h<br>
  include/llvm/IR/Instructions.h<br>
  include/llvm/IR/Operator.h<br>
  lib/IR/ConstantsContext.h<br>
<br>
</blockquote></div></div>