This is an archive of the discontinued LLVM Phabricator instance.

LLVM Cleanup for -Wdeprecated-copy
Needs RevisionPublic

Authored by rtrieu on Oct 3 2018, 10:31 PM.

Details

Reviewers
dexonsmith

Diff Detail

Event Timeline

rtrieu created this revision.Oct 3 2018, 10:31 PM
craig.topper added inline comments.
lib/MC/StringTableBuilder.cpp
27

So you can't have an out of line default destructor to hide it from the inliner or to act as vtable anchor?

sberg added a subscriber: sberg.Oct 4 2018, 12:56 AM
dexonsmith requested changes to this revision.Oct 4 2018, 4:21 PM

We shouldn't be dropping vtable anchors like this.

include/llvm/DebugInfo/CodeView/DebugSubsection.h
23

This moves the vtable anchor from the implementation to the header, which bloats object files considerably and causes the symbol to be weak unnecessarily.

36

This moves the vtable anchor.

include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptor.h
31

This moves the vtable anchor.

include/llvm/MC/StringTableBuilder.h
40

This moves the vtable anchor.

include/llvm/ObjectYAML/MachOYAML.h
57

This moves the vtable anchor.

include/llvm/Support/FormatVariadicDetails.h
25

This moves the vtable anchor.

This revision now requires changes to proceed.Oct 4 2018, 4:21 PM

https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers

Well, learned something new today. I'll follow up with something that doesn't mess with the vtable anchors.

nhaehnle removed a subscriber: nhaehnle.Oct 5 2018, 6:22 AM

https://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers

Well, learned something new today. I'll follow up with something that doesn't mess with the vtable anchors.

Bonus points if you can *add* vtable anchors to the classes that are missing them :).

rtrieu added a comment.Oct 9 2018, 7:57 PM

Looking through other classes, I see that it is common to use an anchor() method as the vtable anchor. I think having it that way makes it more clear what the purpose of the out-of-line function definition is.

include/llvm/DebugInfo/PDB/Native/DbiModuleDescriptor.h
31

None of these methods are virtual, so this shouldn't affect the vtable anchor or am I misreading the coding standard?