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

Update deprecated builtins __has_trivial to __is_trivially #1277

Closed
wants to merge 1 commit into from

Conversation

aaronmondal
Copy link

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 #1201.

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.
@aaronmondal
Copy link
Author

Well that seems to fail 🤣
@derekmauro I tested this with Clang and GCC via bazel test ... Is there some other command i need to run to test this?

@aaronmondal
Copy link
Author

Maybe tests are not actually failing but the CI servers are just unreachable. At least the Details links just give me fusion2’s server IP address could not be found.

@derekmauro
Copy link
Member

derekmauro commented Sep 15, 2022

CI is working correctly, but the results are IP blocked. (There is a way to make the results public, but I haven't had time to figure out how to do that.)

I actually thought cfe27e7 fixed this issue. The remaining uses of __has_trivial should be preprocessor guarded. What platform is demonstrating the issue that this change addresses?

I think the issue with your change is that some of the __is_trivially functions take two arguments. If I recall correctly, the person who committed cfe27e7 said that the simpler fix (the one this change is proposing) had some tricky problems, so we decided to just preprocessor guard the old code.

@aaronmondal
Copy link
Author

@derekmauro The issue arose with Upstream Clang/LLVM, but we were using an abseil version before cfe27e7.

I'll see if cfe27e7 already fixes the issue. If so, I'll close this PR.

Regardless, I think it is strange that bazel test ... passes. I would expect obvious errors such as wrong argcounts to immediately be detected by it.

@aaronmondal
Copy link
Author

cfe27e7 already fixed the issue. Cool! Closing.

keith added a commit to keith/abseil-cpp that referenced this pull request Oct 4, 2022
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
@keith
Copy link
Contributor

keith commented Oct 4, 2022

There seem to be a few more cases here #1289

qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this pull request 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 pull request 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 pull request 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
netkex pushed a commit to netkex/abseil-cpp that referenced this pull request 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants