[llvm] r338175 - [Support] Remove unnecessary MemoryBuffer::anchor (where the destructor serves as the key function)

[David Blaikie via llvm-commits]

Ah, thanks - not sure why I didn't spot that when I was looking earlier.

Yeah, you'd have to go back and check the original review
threads/discussions when the anchors went in I suppose.

I doubt the performance matters terribly either way myself, was just a bit
easier to commit these as explicit anchors so it was clear they made no
change to performance, optimizability, etc.

On Mon, Jul 30, 2018 at 12:57 PM Fāng-ruì Sòng <maskray at google.com> wrote:

> > Oh, looks like the key function/weak vtable issue was removed from the
> style guide at some point - maybe we can/should just rip out all the
> explicit anchors, then?
>
> Is it still there at
>
> https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
> .
> There is no wording about prefererence of dtor or an explicit
> 'anchor()'. For MemoryBuffer, the out-of-line dtors is already there
> `virtual ~MemoryBuffer();` and removing the unnecessary `anchor()`
> might be fine?
> On Mon, Jul 30, 2018 at 12:48 PM David Blaikie <dblaikie at gmail.com> wrote:
> >
> > What's the advantage of moving the dtor to be out of line and act as a
> key function? I Think when I originally added a bunch of these Chris
> Lattner mentioned a slight preference towards an explicit anchor rather
> than placing the dtor out of line (if it wasn't already) - since the latter
> might inhibit optimizations like inlining of the dtor in some cases (even
> with a virtual dtor - not all dtor calls might be virtual, and derived
> dtors need to call the base dtor, etc).
> >
> > Oh, looks like the key function/weak vtable issue was removed from the
> style guide at some point - maybe we can/should just rip out all the
> explicit anchors, then?
> >
> > On Fri, Jul 27, 2018 at 4:12 PM Fangrui Song via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >>
> >> Author: maskray
> >> Date: Fri Jul 27 16:12:11 2018
> >> New Revision: 338175
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=338175&view=rev
> >> Log:
> >> [Support] Remove unnecessary MemoryBuffer::anchor (where the destructor
> serves as the key function)
> >>
> >> Modified:
> >>     llvm/trunk/include/llvm/Support/MemoryBuffer.h
> >>     llvm/trunk/include/llvm/Support/SmallVectorMemoryBuffer.h
> >>     llvm/trunk/lib/Support/MemoryBuffer.cpp
> >>
> >> Modified: llvm/trunk/include/llvm/Support/MemoryBuffer.h
> >> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/MemoryBuffer.h?rev=338175&r1=338174&r2=338175&view=diff
> >>
> ==============================================================================
> >> --- llvm/trunk/include/llvm/Support/MemoryBuffer.h (original)
> >> +++ llvm/trunk/include/llvm/Support/MemoryBuffer.h Fri Jul 27 16:12:11
> 2018
> >> @@ -43,7 +43,6 @@ class MemoryBuffer {
> >>    const char *BufferStart; // Start of the buffer.
> >>    const char *BufferEnd;   // End of the buffer.
> >>
> >> -
> >>  protected:
> >>    MemoryBuffer() = default;
> >>
> >> @@ -148,9 +147,6 @@ public:
> >>    virtual BufferKind getBufferKind() const = 0;
> >>
> >>    MemoryBufferRef getMemBufferRef() const;
> >> -
> >> -private:
> >> -  virtual void anchor();
> >>  };
> >>
> >>  /// This class is an extension of MemoryBuffer, which allows
> copy-on-write
> >>
> >> Modified: llvm/trunk/include/llvm/Support/SmallVectorMemoryBuffer.h
> >> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/SmallVectorMemoryBuffer.h?rev=338175&r1=338174&r2=338175&view=diff
> >>
> ==============================================================================
> >> --- llvm/trunk/include/llvm/Support/SmallVectorMemoryBuffer.h (original)
> >> +++ llvm/trunk/include/llvm/Support/SmallVectorMemoryBuffer.h Fri Jul
> 27 16:12:11 2018
> >> @@ -49,6 +49,9 @@ public:
> >>      init(this->SV.begin(), this->SV.end(), false);
> >>    }
> >>
> >> +  // Key function.
> >> +  ~SmallVectorMemoryBuffer() override;
> >> +
> >>    StringRef getBufferIdentifier() const override { return BufferName; }
> >>
> >>    BufferKind getBufferKind() const override { return
> MemoryBuffer_Malloc; }
> >> @@ -56,7 +59,6 @@ public:
> >>  private:
> >>    SmallVector<char, 0> SV;
> >>    std::string BufferName;
> >> -  void anchor() override;
> >>  };
> >>
> >>  } // namespace llvm
> >>
> >> Modified: llvm/trunk/lib/Support/MemoryBuffer.cpp
> >> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/MemoryBuffer.cpp?rev=338175&r1=338174&r2=338175&view=diff
> >>
> ==============================================================================
> >> --- llvm/trunk/lib/Support/MemoryBuffer.cpp (original)
> >> +++ llvm/trunk/lib/Support/MemoryBuffer.cpp Fri Jul 27 16:12:11 2018
> >> @@ -533,5 +533,4 @@ MemoryBufferRef MemoryBuffer::getMemBuff
> >>    return MemoryBufferRef(Data, Identifier);
> >>  }
> >>
> >> -void MemoryBuffer::anchor() {}
> >> -void SmallVectorMemoryBuffer::anchor() {}
> >> +SmallVectorMemoryBuffer::~SmallVectorMemoryBuffer() {}
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
> --
> 宋方睿
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180730/2e267547/attachment.html>