Page MenuHomePhabricator


Authored by MaskRay on Jan 25 2020, 4:51 PM.



In 2012 (rL168694), it was considered "never used and has bit-rotted".
Now is 2020.

Diff Detail

Unit TestsFailed

2,170 mslibc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_recursive::Unknown Unit Message ("")
Compiled With: '/usr/bin/clang++ -o /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.recursive/Output/try_lock_until.pass.cpp.o -x c++ /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.recursive/try_lock_until.pass.cpp -c -v -ftemplate-depth=270 -Werror=thread-safety -std=c++2a -include /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/nasty_macros.h -nostdinc++ -I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/include -I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/include/c++build -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support -DLIBCXX_FILESYSTEM_STATIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/std/input.output/filesystems/Inputs/static_test_env" -DLIBCXX_FILESYSTEM_DYNAMIC_TEST_ROOT="/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/filesystem/Output/dynamic_env" -DLIBCXX_FILESYSTEM_DYNAMIC_TEST_HELPER="/usr/bin/python /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/libcxx/test/support/" -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -Wall -Wextra -Werror -Wuser-defined-warnings -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -c && /usr/bin/clang++ -o /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/build/projects/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.recursive/Output/try_lock_until.pass.cpp.exe…

Event Timeline

MaskRay created this revision.Jan 25 2020, 4:51 PM

Unit tests: fail. 62194 tests passed, 1 failed and 815 were skipped.

failed: libc++.std/thread/thread_mutex/thread_mutex_requirements/thread_timedmutex_requirements/thread_timedmutex_recursive/try_lock_until.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

mehdi_amini added inline comments.Jan 25 2020, 7:36 PM

Isn't this breaking bitcode backward compatibility?

void added inline comments.Mar 29 2020, 7:46 PM

This used to be okay when changing major versions. I'm not too familiar with the current versioning system though or whether that's still the policy.

mehdi_amini added inline comments.Mar 29 2020, 9:45 PM

It changed when we changed the version numbering to not have major/minor anymore. At the time (IIRC) we considered we would break compatibility only if it is really motivated. The current section of the developer policy:

MaskRay marked an inline comment as done.Apr 11 2020, 9:29 PM
MaskRay added inline comments.

Is this a case that "really no one makes use of the feature"? It was planned to be removed in 4.0

mehdi_amini added inline comments.Apr 12 2020, 10:37 AM

Anything that was "planned to be removed in 4.0" is now supported "forever" until we make an explicit choice to break backward compatibility.

The reason is that "4.0" was used during the development of the 3.x versions as "this future major breaking change version". At the time the major number was used to indicate the compatibility. When we reached 3.9 we decided to change the numbering, instead of going to 3.10 we went to 4.0 but after changing the meaning of the major number to not mean anything anymore with respect to bitcode backward compatibility.

dexonsmith requested changes to this revision.Jun 24 2020, 3:17 PM

Anything that was "planned to be removed in 4.0" is now supported "forever" until we make an explicit choice to break backward compatibility.

As Mehdi pointed out, the right fix is probably to update the comment(s) to something like:

// Deprecated, but still needed to read old bitcode files.
This revision now requires changes to proceed.Jun 24 2020, 3:17 PM
MaskRay abandoned this revision.Jun 24 2020, 6:24 PM