Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

std::aligned_storage is deprecated as of C++23 #1908

Conversation

andrewcorrigan
Copy link
Contributor

@andrewcorrigan andrewcorrigan commented Mar 27, 2023

std::aligned_storage is deprecated as of C++23. Compiling with gcc-13 and -std=c++2b, I get warnings like,

/thrust/thrust/cmake/../../thrust/detail/alignment.h:167:34: warning: ‘template<long unsigned int _Len, long unsigned int _Align> struct std::aligned_storage’ is deprecated [-Wdeprecated-declarations]
  167 |     using aligned_storage = std::aligned_storage<Len, Align>;

from innocuous code that simply include a Thrust header file

#include <thrust/device_vector.h>
  
int main(int argc, char** argv)
{
    return 0;
}

Updated Approach

This PR now proposes to eliminate thrust::detail::aligned_storage and the associated tests.

Original Approach

I first tried a surgical change like the following

#if THRUST_CPP_DIALECT >= 2011 && THRUST_CPP_DIALECT <= 2020
    template <std::size_t Len, std::size_t Align>
    using aligned_storage = std::aligned_storage<Len, Align>;
#else

but the warning persisted.

Therefore, the proposed resolution in this PR simply deletes the THRUST_CPP_DIALECT >= 2011 implementation:

#if THRUST_CPP_DIALECT >= 2011
    template <std::size_t Len, std::size_t Align>
    using aligned_storage = std::aligned_storage<Len, Align>;
#else

unconditionally switching back to the pre-C++11 implementation.

@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

Admins can comment ok to test to allow this one PR to run or add to allowlist to allow all future PRs from the same author to run.

Copy link
Collaborator

@miscco miscco left a comment

Choose a reason for hiding this comment

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

I am decidedly not happy with this patch. std::aligned_storage has been deprecated for a reason. It does not serve any purpose anymore and is both easy to miss-use and hard to use. Any use of std::aligned_storage is better replaced by

alignas(T) std::byte buffer[sizeof(T)];

So the proper solution to the deprecation of std::aligned_storage is to stop using it. Note that depending on your standard library you can always simply silence the deprecation warning.

I am strongly leaning towards closing this but will wait for another maintainers opinion

@andrewcorrigan
Copy link
Contributor Author

If you prefer a different approach such as eliminating entirely, I’d be happy to redo the PR.

@miscco
Copy link
Collaborator

miscco commented Mar 27, 2023

If you prefer a different approach such as eliminating entirely, I’d be happy to redo the PR.

My preferred solution would be to do nothing.

A user can properly fix their code or add a compiler flag. There is no useful action on our side as the issue is not that we are using deprecated facilities, but that the whole feature is deprecated

@andrewcorrigan
Copy link
Contributor Author

I'm confused now: doing nothing means using a deprecated feature and Thrust triggering warnings by default. There's nothing in user code triggering said warnings, other than including a Thrust header. Is the solution you prefer really to have users enable flags to suppress warnings for code that will presumably become an error when this feature is eventually removed from the standard? As you say the whole feature is deprecated why not eliminate it, both the C++11 and fallback version, from Thrust?

@andrewcorrigan
Copy link
Contributor Author

I just verified that eliminating all implementations of thrust::detail::aligned_storage works: eliminating this deprecated feature from Thrust eliminates the warning I posted above from innocuous user code like,

#include <thrust/device_vector.h>
  
int main(int argc, char** argv)
{
    return 0;
}

I also eliminated the corresponding tests. I'm not able to push this morning, but I will try pushing again later.

@andrewcorrigan andrewcorrigan force-pushed the cxx23-aligned_storage-deprecated branch from 4b92de3 to 06cbce4 Compare March 27, 2023 13:25
@andrewcorrigan
Copy link
Contributor Author

@allisonvacanti would you mind taking a look at this as well? I'm curious if you have any feedback.

To summarize the above:

Compiling Thrust code using C++23 currently triggers warnings (or errors if compiled with -Werror). The deprecated functionality currently in Thrust is included unconditionally even for the trivial user code I posted above. This deprecated functionality does not appear to be used, so this PR proposes to eliminate it.

@miscco
Copy link
Collaborator

miscco commented Jul 24, 2023

Hi @andrewcorrigan I am closing this in favor of NVIDIA/cccl#258 as we have moved our repositories to a unified monorepo

I have adopted your changes, to simply ignore the deprecation warning for now. That does not break users which require thrust::aligned_storage but helps build with c++23.

It would be awesome if you could test whether this works for your project

@miscco miscco closed this Jul 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants