Page MenuHomePhabricator

[libc++] Do not force the use of -Werror in verify tests
ClosedPublic

Authored by ldionne on Mar 17 2020, 12:05 PM.

Details

Reviewers
EricWF
Group Reviewers
Restricted Project
Commits
rGa5fa5f7cb86c: [libc++] Do not force the use of -Werror in verify tests
Summary

Forcing -Werror and other warnings means that the test suite isn't
actually testing what most people are seeing in their code -- it seems
better and less arbitrary to compile these tests as close as possible
to the compiler default instead.

Removing -Werror also means that we get to differentiate between
diagnostics that are errors and those that are warnings, which makes
the test suite more precise.

Diff Detail

Event Timeline

ldionne created this revision.Mar 17 2020, 12:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2020, 12:05 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
EricWF accepted this revision.Mar 18 2020, 8:00 PM
EricWF added a subscriber: EricWF.

I'm not sure I love this, but I'm OK demoting warnings to errors in the XFAIL tests.

libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/reset.runtime.fail.cpp
10

Why does this no longer pass?

This revision is now accepted and ready to land.Mar 18 2020, 8:00 PM
ldionne marked 2 inline comments as done.Mar 18 2020, 9:00 PM
ldionne added inline comments.
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/reset.runtime.fail.cpp
10

When adding -Werror, we were also adding some specially tailored -Wno-c++11-extensions warning suppressor. The problem is that we use min_allocator.h, which uses the C++11 extension = default even in C++03 mode.

My original workaround was to backport min_allocator.h to C++03, but I thought you would prefer just disabling that specific test (unique_ptr is supposed to be C++11 anyway).

This revision was automatically updated to reflect the committed changes.
ldionne marked an inline comment as done.
MaskRay added a subscriber: MaskRay.EditedMar 26 2020, 5:36 PM

Just out of curiosity, did you create the patch by writing a script to parse clang -verify output and fix the tests automatically?
I asked because I had done similar things.

EricWF added inline comments.Mar 26 2020, 6:28 PM
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.modifiers/reset.runtime.fail.cpp
10

FYI we do use unique_ptr throughout our implementation of the C++03 library.

Just out of curiosity, did you create the patch by writing a script to parse clang -verify output and fix the tests automatically?
I asked because I had done similar things.

No, I am writing a new test format for libc++ and it wasn't adding that specific warning flag. I noticed the failures of the tests changed by this patch, observed it was because we didn't pass the warning flag, and created this patch.