This is an archive of the discontinued LLVM Phabricator instance.

Deleted old fixme.
ClosedPublic

Authored by Prazek on Aug 10 2015, 9:25 PM.

Details

Diff Detail

Event Timeline

Prazek updated this revision to Diff 31767.Aug 10 2015, 9:25 PM
Prazek retitled this revision from to Small fixup.
Prazek updated this object.
Prazek added reviewers: rsmith, majnemer.
Prazek added a subscriber: cfe-commits.
rsmith added a reviewer: rnk.Aug 11 2015, 2:09 PM
rsmith edited edge metadata.

Reid, do you remember what this FIXME was for? (Added in r198462.)

rnk edited edge metadata.Aug 11 2015, 2:15 PM

I added this code before C++11 migration. I wanted to explicitly default the copy ctor to document that it's a value type, rather than defaulting it implicitly. You can nuke both comments and uncomment the definition.

Prazek updated this revision to Diff 31863.Aug 11 2015, 2:27 PM
Prazek edited edge metadata.

This change now disables the move constructor and move assignment. I think just removing the comment is better.

rnk added inline comments.Aug 11 2015, 3:28 PM
include/clang/AST/VTableBuilder.h
54

Is this ctor used? It leaves Value uninitialized. Maybe we should delete it to ensure that it isn't called, or use a default member initializer for Value below.

Prazek added inline comments.Aug 11 2015, 3:44 PM
include/clang/AST/VTableBuilder.h
54

it is required, f.e.:

VTableComponents(new VTableComponent[NumVTableComponents]),

Hi Piotr,

Would you mind renaming the title of the Patch since the title will be
committed as such?

Thanks!

Prazek updated this revision to Diff 31891.Aug 11 2015, 5:33 PM
Prazek retitled this revision from Small fixup to Deleted old fixme..
rnk accepted this revision.Aug 11 2015, 5:39 PM
rnk edited edge metadata.

lgtm

This revision is now accepted and ready to land.Aug 11 2015, 5:39 PM
Prazek closed this revision.Aug 12 2015, 11:42 AM