Skip to content

[libc++][type_traits] Implements "A type trait to detect reference binding to temporary" #128649

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented Feb 25, 2025

Implements partially: P2255R2: A type trait to detect reference binding to temporary
Issue: #105180

https://eel.is/c++draft/meta.type.synop
https://eel.is/c++draft/meta.unary.prop

Implented type traits:

  • reference_constructs_from_temporary
  • reference_converts_from_temporary

Closes #129049

Minor drive-by tweak to std::is_implicit_lifetime tests.

Copy link

github-actions bot commented Feb 25, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/P2255R2-A_type_trait_to_detect_reference_binding_to_temporary branch 8 times, most recently from 8be15ab to bf9e5d9 Compare February 25, 2025 10:26
…nding to temporary"

Implements partially: P2255R2: A type trait to detect reference binding to temporary
llvm#105180

https://eel.is/c++draft/meta.type.synop
https://eel.is/c++draft/meta.unary.prop

Implented type traits:
- [x] `reference_constructs_from_temporary`
- [x] `reference_converts_from_temporary`
@H-G-Hristov H-G-Hristov force-pushed the hgh/libcxx/P2255R2-A_type_trait_to_detect_reference_binding_to_temporary branch from bf9e5d9 to ab21883 Compare February 25, 2025 11:38
@H-G-Hristov
Copy link
Contributor Author

Removed workarounds and disabled tests for unsupported compilers:

According to #111477 __has_builtin(reference_{constructs|converts|_from_temporary) was broken in Clang 19.1.0 and it was fixed in Clang 19.1.2 with dedbdfb. Workarounds were implemented and test for unsupported compilers disabled.

@philnik777
Copy link
Contributor

If only the check was broken, can't we just drop the check entirely?

@H-G-Hristov
Copy link
Contributor Author

If only the check was broken, can't we just drop the check entirely?

Wouldn't this cause build failures in the CI if the the intrinsic isn't available?

@philnik777
Copy link
Contributor

If only the check was broken, can't we just drop the check entirely?

Wouldn't this cause build failures in the CI if the the intrinsic isn't available?

When is it not available?

@H-G-Hristov
Copy link
Contributor Author

If only the check was broken, can't we just drop the check entirely?

Wouldn't this cause build failures in the CI if the the intrinsic isn't available?

When is it not available?

On Arm currently, Apple Clang

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Some stuffs in the test code are confusing, but I think we can do the cleanup later.

@philnik777 philnik777 merged commit 6d9dfd7 into llvm:main Mar 8, 2025
84 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 8, 2025

LLVM Buildbot has detected a new failure on builder sanitizer-aarch64-linux-bootstrap-ubsan running on sanitizer-buildbot10 while building libcxx at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/85/builds/6282

Here is the relevant piece of the build log for the reference
Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using lld-link: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/lld-link
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using ld64.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using wasm-ld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using ld.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld.lld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using lld-link: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/lld-link
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using ld64.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using wasm-ld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 86976 tests, 72 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 
FAIL: LLVM-Unit :: Support/./SupportTests/47/88 (79920 of 86976)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/47/88' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/unittests/Support/./SupportTests-LLVM-Unit-1753793-47-88.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=88 GTEST_SHARD_INDEX=47 /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/unittests/Support/./SupportTests
--

Script:
--
/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/unittests/Support/./SupportTests --gtest_filter=Caching.WriteAfterCommit
--
/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/unittests/Support/Caching.cpp:110: Failure
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /tmp/lit-tmp-kofqsd1m/llvm_test_cache/LLVMTest-18f67a.tmp.o to /tmp/lit-tmp-kofqsd1m/llvm_test_cache/llvmcache-foo: No such file or directory
)


/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/unittests/Support/Caching.cpp:110
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /tmp/lit-tmp-kofqsd1m/llvm_test_cache/LLVMTest-18f67a.tmp.o to /tmp/lit-tmp-kofqsd1m/llvm_test_cache/llvmcache-foo: No such file or directory
)



********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Slowest Tests:
--------------------------------------------------------------------------
121.92s: Clang :: Driver/fsanitize.c
83.58s: Clang :: Driver/arm-cortex-cpus-2.c
82.33s: Clang :: Driver/arm-cortex-cpus-1.c
81.90s: Clang :: Preprocessor/riscv-target-features.c
78.03s: Clang :: OpenMP/target_defaultmap_codegen_01.cpp
75.25s: Clang :: OpenMP/target_update_codegen.cpp
63.91s: Clang :: Preprocessor/aarch64-target-features.c
62.52s: Clang :: Preprocessor/arm-target-features.c
58.06s: Clang :: Driver/clang_f_opts.c
Step 11 (stage2/ubsan check) failure: stage2/ubsan check (failure)
...
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using lld-link: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/lld-link
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using ld64.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using wasm-ld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using ld.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld.lld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using lld-link: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/lld-link
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using ld64.lld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/ld64.lld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/llvm/config.py:512: note: using wasm-ld: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/wasm-ld
llvm-lit: /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 86976 tests, 72 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 
FAIL: LLVM-Unit :: Support/./SupportTests/47/88 (79920 of 86976)
******************** TEST 'LLVM-Unit :: Support/./SupportTests/47/88' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/unittests/Support/./SupportTests-LLVM-Unit-1753793-47-88.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=88 GTEST_SHARD_INDEX=47 /home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/unittests/Support/./SupportTests
--

Script:
--
/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm_build_ubsan/unittests/Support/./SupportTests --gtest_filter=Caching.WriteAfterCommit
--
/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/unittests/Support/Caching.cpp:110: Failure
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /tmp/lit-tmp-kofqsd1m/llvm_test_cache/LLVMTest-18f67a.tmp.o to /tmp/lit-tmp-kofqsd1m/llvm_test_cache/llvmcache-foo: No such file or directory
)


/home/b/sanitizer-aarch64-linux-bootstrap-ubsan/build/llvm-project/llvm/unittests/Support/Caching.cpp:110
Value of: llvm::detail::TakeError(CFS->commit())
Expected: succeeded
  Actual: failed  (Failed to rename temporary file /tmp/lit-tmp-kofqsd1m/llvm_test_cache/LLVMTest-18f67a.tmp.o to /tmp/lit-tmp-kofqsd1m/llvm_test_cache/llvmcache-foo: No such file or directory
)



********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Slowest Tests:
--------------------------------------------------------------------------
121.92s: Clang :: Driver/fsanitize.c
83.58s: Clang :: Driver/arm-cortex-cpus-2.c
82.33s: Clang :: Driver/arm-cortex-cpus-1.c
81.90s: Clang :: Preprocessor/riscv-target-features.c
78.03s: Clang :: OpenMP/target_defaultmap_codegen_01.cpp
75.25s: Clang :: OpenMP/target_update_codegen.cpp
63.91s: Clang :: Preprocessor/aarch64-target-features.c
62.52s: Clang :: Preprocessor/arm-target-features.c
58.06s: Clang :: Driver/clang_f_opts.c

@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/P2255R2-A_type_trait_to_detect_reference_binding_to_temporary branch March 8, 2025 17:13
@frederick-vs-ja
Copy link
Contributor

I guess we need to revert the drive-by changes. Not sure whether that would be temporary.

At the time of last CI for this PR, the "macos (apple-configuration, macos-15)" configuration was using apple-clang-16. But today it changed to use apple-clang-17 and the tests for is_implicit_lifetime still failed.

frederick-vs-ja added a commit that referenced this pull request Mar 14, 2025
…-17 (#131302)

The skipping was present before but dropped. At the time it was dropped
in #128649, apple-clang-17 wasn't used for CI. But today it is used at
least for the "macos (generic-cxx23, macos-15)" configuration. So I
think we need to skip apple-clang-17 again.
: public bool_constant<__reference_constructs_from_temporary(_Tp, _Up)> {};

template <class _Tp, class _Up>
_LIBCPP_NO_SPECIALIZATIONS inline constexpr bool reference_constructs_from_temporary_v =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zingam It looks like you forgot to add tests for the _LIBCPP_NO_SPECIALIZATIONS annotations. Could you add them in a follow-up PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++23 libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] P2255R2: Implement type traits
6 participants