Skip to content
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

abseil's absl::is_trivially_copy_assignable uses deprecated, broken compiler builtin #1201

Closed
royjacobson opened this issue Jun 22, 2022 · 3 comments · Fixed by #1289
Closed
Assignees
Labels

Comments

@royjacobson
Copy link

royjacobson commented Jun 22, 2022

As a side effect of implementing DR2171 for clang, we have discovered that absl::is_trivially_copy_assignable uses the __has_trivial_assign compiler builtin.

This compiler builtin has bad semantics when deleted functions are involved because clang considers them trivial for this purpose. Specifically it appears that MSVC stl delete the copy assignment operator for volatile& std::pair which causes abseil to assert when instantiating absl::Optional<std::pair<int, int>> with MSVC stl.

This has been discussed over at https://reviews.llvm.org/D127593. We might revert this change momentarily, but it would be helpful if abseil will stop using the __has_trivial... builtins.

Note that GCC uses the same 'deleted is trivial' semantics as clang, and that those builtins have been deprecated for quite some time: this is a discussion of the same issue from 2017: llvm/llvm-project#33063

@derekmauro derekmauro self-assigned this Jun 23, 2022
royjacobson added a commit to llvm/llvm-project that referenced this issue Jul 12, 2022
…pe traits

Some compiler provided type traits like __has_trivial_constructor have been documented
as deprecated for quite some time.
Still, some people apparently still use them, even though mixing them with concepts
and with deleted functions leads to weird results. There's also disagreement about some
edge cases between GCC (which Clang claims to follow) and MSVC.

This patch adds deprecation warnings for the usage of those builtins, except for __has_trivial_destructor
which doesn't have a GCC alternative.

I made the warning on by default, so I had to silence it for some tests but it's not too many.

Some (decade old) history of issues with those builtins:
#18187
#18559
#22161
#33063

The abseil usage of them that triggered me to add this warning:
abseil/abseil-cpp#1201

Weird interaction of those builtins with C++20's conditionally trivial special member functions:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106085

Reviewed By: #clang-language-wg, aaron.ballman

Differential Revision: https://reviews.llvm.org/D129170
@sylvestre
Copy link

On the Mozilla side: https://bugzilla.mozilla.org/show_bug.cgi?id=1779528 (firefox uses libwebrtc which uses abseil-cpp)

@sseckler
Copy link

sseckler commented Jul 21, 2022

In addition to __has_trivial_assign, abseil uses __has_trivial_copy and __has_trivial_constructor in type_traits.h which are also deprecated.

aaronmondal added a commit to aaronmondal/abseil-cpp that referenced this issue Sep 15, 2022
The old builtins have been depracated in Clang 16. Since they are
also present in GCC it should be fine to just use the new variants.

Fixes Issue abseil#1201.
@keith
Copy link
Contributor

keith commented Oct 4, 2022

#1289

mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
…pe traits

Some compiler provided type traits like __has_trivial_constructor have been documented
as deprecated for quite some time.
Still, some people apparently still use them, even though mixing them with concepts
and with deleted functions leads to weird results. There's also disagreement about some
edge cases between GCC (which Clang claims to follow) and MSVC.

This patch adds deprecation warnings for the usage of those builtins, except for __has_trivial_destructor
which doesn't have a GCC alternative.

I made the warning on by default, so I had to silence it for some tests but it's not too many.

Some (decade old) history of issues with those builtins:
llvm/llvm-project#18187
llvm/llvm-project#18559
llvm/llvm-project#22161
llvm/llvm-project#33063

The abseil usage of them that triggered me to add this warning:
abseil/abseil-cpp#1201

Weird interaction of those builtins with C++20's conditionally trivial special member functions:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106085

Reviewed By: #clang-language-wg, aaron.ballman

Differential Revision: https://reviews.llvm.org/D129170
copybara-service bot pushed a commit to google-deepmind/mujoco that referenced this issue Nov 4, 2022
Since upgrading to Clang 15, abseil fails to build, because of abseil/abseil-cpp#1201.

PiperOrigin-RevId: 486190797
Change-Id: Ibaa1c03b8d21ae4a83c3825c8d56363aa2f89505
copybara-service bot pushed a commit to protocolbuffers/upb that referenced this issue Jan 14, 2023
copybara-service bot pushed a commit to protocolbuffers/upb that referenced this issue Jan 14, 2023
copybara-service bot pushed a commit to protocolbuffers/upb that referenced this issue Jan 14, 2023
qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this issue Feb 1, 2023
If you compile with clang 15+, the uses of trivially destructible and
assignable are deprecated. This sets this configuration correctly as the
ifdef to fix the build.

Fixes abseil/abseil-cpp#1201
Related abseil/abseil-cpp#1277

Original Pull Request:
abseil/abseil-cpp#1289

Task-number: QTBUG-108240
Change-Id: Id5456e3da01e16e9370f9fa6ed279360e1df523d
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/455716
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
razmser pushed a commit to razmser/abseil-cpp that referenced this issue Sep 12, 2023
If you compile with clang 15+, the uses of trivially destructible and
assignable are deprecated. This sets this configuration correctly as the
ifdef to fix the build.

Fixes abseil#1201
Related abseil#1277
razmser pushed a commit to razmser/abseil-cpp that referenced this issue Sep 12, 2023
If you compile with clang 15+, the uses of trivially destructible and
assignable are deprecated. This sets this configuration correctly as the
ifdef to fix the build.

Fixes abseil#1201
Related abseil#1277
clint-stripe added a commit to clint-stripe/livegrep that referenced this issue Oct 5, 2023
macOS Sonoma brought along a new Xcode version, which broke some things.

We need to pick up abseil/abseil-cpp#1201
clintharrison added a commit to clintharrison/livegrep that referenced this issue Oct 5, 2023
macOS Sonoma brought along a new Xcode version, which broke some things.

We need to pick up abseil/abseil-cpp#1201
clintharrison added a commit to clintharrison/livegrep that referenced this issue Oct 5, 2023
macOS Sonoma brought along a new Xcode version, which broke some things.

We need to pick up abseil/abseil-cpp#1201
nelhage pushed a commit to livegrep/livegrep that referenced this issue Oct 7, 2023
* Upgrade abseil for Clang 15 (macOS Sonoma) compatibility

macOS Sonoma brought along a new Xcode version, which broke some things.

We need to pick up abseil/abseil-cpp#1201

* Move to hedron_compile_commands, add vscode/gopls settings

* Update proto to support multiple path/-paths

* Support multiple paths in backend

* Parse multiple file: ops in the web server

* Add C++ version to .bazelrc.ci
wfrisch pushed a commit to wfrisch/livegrep that referenced this issue Nov 14, 2023
* Upgrade abseil for Clang 15 (macOS Sonoma) compatibility

macOS Sonoma brought along a new Xcode version, which broke some things.

We need to pick up abseil/abseil-cpp#1201

* Move to hedron_compile_commands, add vscode/gopls settings

* Update proto to support multiple path/-paths

* Support multiple paths in backend

* Parse multiple file: ops in the web server

* Add C++ version to .bazelrc.ci
netkex pushed a commit to netkex/abseil-cpp that referenced this issue Apr 3, 2024
If you compile with clang 15+, the uses of trivially destructible and
assignable are deprecated. This sets this configuration correctly as the
ifdef to fix the build.

Fixes abseil#1201
Related abseil#1277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants