[PATCH] D55834: [libcxx] Make sure all experimental tests are disabled when enable_experimental=False

Louis Dionne via Phabricator reviews at reviews.llvm.org
Tue Dec 18 10:49:57 PST 2018


ldionne added a comment.

In D55834#1334839 <https://reviews.llvm.org/D55834#1334839>, @EricWF wrote:

> If the tests are header only, why disable them? We are losing code coverage on things people may be depending on.


I don't expect most people to be using `enable_experimental=False`, so that shouldn't actually reduce the test coverage. The problem I hit was running CI on older OS X where a test was failing (because of a problem with exceptions in the old dylib). The experimental part doesn't actually use anything in the dylib, but the test as a whole does, and enabling it when I explicitly say `enable_experimental=False` was surprising certainly not what I expected.

In other words, I fully agree that we should strive to run experimental tests as often as possible (when it makes sense), and we do have the right default here. But when I explicitly say "don't give me experimental tests", I don't want them to run.

If we'd rather not give that option at all, we could consider inferring the `c++experimental` LIT feature automatically based on stuff like whether `use_system_cxx_lib` was used and other things, but I don't think it makes a lot of sense.

WDYT?


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55834/new/

https://reviews.llvm.org/D55834





More information about the libcxx-commits mailing list