Page MenuHomePhabricator

[libcxx] Fix copy/move issues caused by __tuple_leaf's converting constructor.
ClosedPublic

Authored by EricWF on Jul 9 2014, 8:57 PM.

Details

Reviewers
mclow.lists
Summary

This patch contains two fixes issues arising around __tuple_leafs converting constructor.

  1. The first problem is that __tuple_leaf<T> uses is_constructible<T, U> to enable/disable it's converting constructor __tuple_leaf(U &&). Evaluating is_constructible under certain conditions can cause a hard compile error. Currently is_constructible<T, __tuple_leaf> is evaluated whenever __tuple_leaf is copied or moved. This patch prevents that from happening by only evaluating is_constructible when U != __tuple_leaf.
  2. The second problem is that __tuple_leaf's specialization for empty types does have an implicitly generated move constructor because it declares a deleted assignment operator.

This causes the converting to be constructor during moves. The problem is solved by disabling the constructor when U == __tuple_leaf and giving __tuple_leaf a defaulted move constructor (so the defaulted copy constructor is not called).

There is also a small fix to two tests that needed extra brackets around an assert macro.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 11240.Jul 9 2014, 8:57 PM
EricWF retitled this revision from to [libcxx] Fix copy/move issues caused by __tuple_leaf's converting constructor..
EricWF updated this object.
EricWF edited the test plan for this revision. (Show Details)
EricWF added a reviewer: mclow.lists.
EricWF set the repository for this revision to rL LLVM.
EricWF updated this object.
EricWF added a subscriber: Unknown Object (MLST).
EricWF updated this object.Jul 9 2014, 9:00 PM
EricWF edited the test plan for this revision. (Show Details)
EricWF updated this object.
EricWF updated this revision to Diff 11241.Jul 9 2014, 9:05 PM

Uploaded the correct version of the patch. Sorry for the spam.

mclow.lists accepted this revision.Jul 24 2014, 8:42 AM
mclow.lists edited edge metadata.

LGTM. Thanks!
Do you need me to commit this?

This revision is now accepted and ready to land.Jul 24 2014, 8:42 AM
EricWF closed this revision.Jul 24 2014, 11:44 AM

Urg, "arc commit" is not playing nicely. Manually committed as r213888.