base nocompile tests fail with tip-of-tree clang |
|||||
Issue descriptionFor example here: https://logs.chromium.org/logs/chromium/bb/chromium.clang/ToTLinux/4548/+/recipes/steps/compile/0/stdout #error "NCTEST_UNCHECKED_OBSERVER_USING_CHECKED_LIST Failed: Expectations [r'fatal error: static_assert failed due to requirement 'std::is_base_of<CheckedObserver, UncheckedObserver>::value' \"Observers should inherit from base::CheckedObserver. Use ObserverList<T>::Unchecked to observe with raw pointers.\"'] did not match output." The diagnostic has changed to print the qualified type: gen/base/observer_list_unittest_nc.cc:15:2: error: " ../../base/observer_list_internal.h:85:5: fatal error: static_assert failed due to requirement 'std::is_base_of<base::CheckedObserver, UncheckedObserver>::value' "Observers should inherit from base::CheckedObserver. Use ObserverList<T>::Unchecked to observe with raw pointers."" (Note the extra base:: in front of CheckedObersver) This changed here: ------------------------------------------------------------------------ r348239 | courbet | 2018-12-04 08:59:57 +0100 (Tue, 04 Dec 2018) | 25 lines [WIP][Sema] Improve static_assert diagnostics for type traits. Summary: In our codebase, `static_assert(std::some_type_trait<Ts...>::value, "msg")` (where `some_type_trait` is an std type_trait and `Ts...` is the appropriate template parameters) account for 11.2% of the `static_assert`s. In these cases, the `Ts` are typically not spelled out explicitly, e.g. `static_assert(std::is_same<SomeT::TypeT, typename SomeDependentT::value_type>::value, "message");` The diagnostic when the assert fails is typically not very useful, e.g. `static_assert failed due to requirement 'std::is_same<SomeT::TypeT, typename SomeDependentT::value_type>::value' "message"` This change makes the diagnostic spell out the types explicitly , e.g. `static_assert failed due to requirement 'std::is_same<int, float>::value' "message"` See tests for more examples. After this is submitted, I intend to handle `static_assert(!std::some_type_trait<Ts...>::value, "msg")`, which is another 6.6% of static_asserts. Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D54903 ------------------------------------------------------------------------
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c9efdb6fb26376a826151d722add7689b0af917d commit c9efdb6fb26376a826151d722add7689b0af917d Author: Hans Wennborg <hans@chromium.org> Date: Wed Dec 05 11:55:57 2018 Revert "gc-plugin: Allow typdef as mixin marker in using macro" This reverts commit 84d989af7b54497c435347b4e9d6c84114c8b5e2. Reason for revert: The test doesn't pass: https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.clang%2FToTLinux%2F4559%2F%2B%2Frecipes%2Fsteps%2Fgclient_runhooks%2F0%2Fstdout Original change's description: > gc-plugin: Allow typdef as mixin marker in using macro > > This allows us to get rid of the empty class marker when we decide to > check construction in a different way. The typedef does not require > memory in contrast to an empty class. > > Bug: 911662 > Change-Id: I99ab7e35e2b60dd0065f0db2483c419ae7b81002 > Reviewed-on: https://chromium-review.googlesource.com/c/1361712 > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Commit-Queue: Michael Lippautz <mlippautz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613915} TBR=haraken@chromium.org,mlippautz@chromium.org Change-Id: Ia777738ea594b1c83cc6d1ab73a315ef0e6812a8 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 911662, 912021 Reviewed-on: https://chromium-review.googlesource.com/c/1362901 Reviewed-by: Hans Wennborg <hans@chromium.org> Commit-Queue: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#613942} [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/BadPatternFinder.cpp [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/tests/heap/stubs.h [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/tests/missing_mixin_marker.cpp [modify] https://crrev.com/c9efdb6fb26376a826151d722add7689b0af917d/tools/clang/blink_gc_plugin/tests/missing_mixin_marker.txt
,
Dec 5
> Revert "gc-plugin: Allow typdef as mixin marker in using macro" That was for crbug.com/912040 , I got the Bug: field wrong.
,
Dec 5
> My attempt to repro locally has failed. Maybe it got fixed? Nope, it's still happening: https://ci.chromium.org/buildbot/chromium.clang/ToTLinux/4561
,
Dec 5
Over to tapted who added this in https://chromium.googlesource.com/chromium/src/+/30f97fddc348e14c23499ffc0f638deff901641c Oh, apparently away until mid-Jan 2019. I think we should just disable the test, and then maybe reenable once we've rolled in the new diag. (Also, nc tests are imho more trouble than they're worth :-/)
,
Dec 6
Disabling the test here: https://chromium-review.googlesource.com/c/chromium/src/+/1365434 The reason I wasn't able to reproduce locally at first is that these tests don't respect clang_base_path.
,
Dec 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8a149309d01fa018dba3f5957be945cb503cbc4 commit b8a149309d01fa018dba3f5957be945cb503cbc4 Author: Hans Wennborg <hans@chromium.org> Date: Thu Dec 06 10:40:36 2018 Disable disable_observer_list_unittest.nc It fails with tip-of-tree Clang because the diagnositics have changed slightly. Disable for now and possible re-enable with updated expectations after the next Clang roll. TBR=thakis Bug: 912021 Change-Id: Ib0d83178ea5c38fa2d19ada720e8526437c916fd Reviewed-on: https://chromium-review.googlesource.com/c/1365434 Reviewed-by: Hans Wennborg <hans@chromium.org> Commit-Queue: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#614306} [modify] https://crrev.com/b8a149309d01fa018dba3f5957be945cb503cbc4/base/BUILD.gn [delete] https://crrev.com/ad030e8d499b927c623fa2715f95fde38c44cdd8/base/observer_list_unittest.nc
,
Dec 7
,
Dec 7
,
Dec 10
More nc tests started failing this morning, see https://ci.chromium.org/buildbot/chromium.clang/ToTLinux/4615 Presumably due to this: ------------------------------------------------------------------------ r348741 | courbet | 2018-12-10 09:19:38 +0100 (Mon, 10 Dec 2018) | 11 lines [Sema] Further improvements to to static_assert diagnostics. Summary: We're now handling cases like `static_assert(!expr)` and static_assert(!(expr))`. Reviewers: aaron.ballman, Quuxplusone Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D55270 ------------------------------------------------------------------------ Looks like it was taken out again in r348742, but it'll be back. I'll disable the tests until we roll and can update the expectations: https://chromium-review.googlesource.com/c/chromium/src/+/1369926
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9c36ded1a275e76b10391e69e1c1a984549316ae commit 9c36ded1a275e76b10391e69e1c1a984549316ae Author: Hans Wennborg <hans@chromium.org> Date: Mon Dec 10 11:01:33 2018 Disable more base nocompile tests The expectations don't match the diagnostics in new Clang versions. Disable until we update the compiler and can change the expectations. TBR=thakis Bug: 912021 Change-Id: I832f8fc26092480aefc6f6ee0b6e43e256c0b4be Reviewed-on: https://chromium-review.googlesource.com/c/1369926 Reviewed-by: Hans Wennborg <hans@chromium.org> Commit-Queue: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#615079} [modify] https://crrev.com/9c36ded1a275e76b10391e69e1c1a984549316ae/base/BUILD.gn [delete] https://crrev.com/6fee454f21e59fed7415b692a542af225fd27d33/base/bind_unittest.nc [delete] https://crrev.com/6fee454f21e59fed7415b692a542af225fd27d33/base/containers/span_unittest.nc [delete] https://crrev.com/6fee454f21e59fed7415b692a542af225fd27d33/base/metrics/histogram_unittest.nc
,
Dec 10
> The reason I wasn't able to reproduce locally at first is that these tests don't respect clang_base_path. Can you file a bug for this and give it to wychen?
,
Dec 10
> Can you file a bug for this and give it to wychen? https://bugs.chromium.org/p/chromium/issues/detail?id=913420
,
Dec 10
Adding metrics component since the CL under review (https://chromium-review.googlesource.com/c/chromium/src/+/1369926) is disabling some metrics tests.
,
Jan 7
I have a CL to re-enable the tests at https://chromium-review.googlesource.com/c/chromium/src/+/1388818, but Hans is out for another week.
,
Jan 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e5d14a874ac175c2014d3538908ed227228f1706 commit e5d14a874ac175c2014d3538908ed227228f1706 Author: Hans Wennborg <hans@chromium.org> Date: Mon Jan 14 11:31:24 2019 Reenable nocompile tests after clang roll. Bug: 912021 Change-Id: Ifbdad47d102bb2470c236bc51b3fdbf689fef4d7 Reviewed-on: https://chromium-review.googlesource.com/c/1388818 Commit-Queue: Hans Wennborg <hans@chromium.org> Reviewed-by: Hans Wennborg <hans@chromium.org> Cr-Commit-Position: refs/heads/master@{#622422} [modify] https://crrev.com/e5d14a874ac175c2014d3538908ed227228f1706/base/BUILD.gn [add] https://crrev.com/e5d14a874ac175c2014d3538908ed227228f1706/base/bind_unittest.nc [add] https://crrev.com/e5d14a874ac175c2014d3538908ed227228f1706/base/containers/span_unittest.nc [add] https://crrev.com/e5d14a874ac175c2014d3538908ed227228f1706/base/metrics/histogram_unittest.nc [add] https://crrev.com/e5d14a874ac175c2014d3538908ed227228f1706/base/observer_list_unittest.nc
,
Jan 14
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by h...@chromium.org
, Dec 5