It seems std::is_literal_type was removed from C++ 20, so now GCC is warning that we use it in wtf/Variant.h: DerivedSources/ForwardingHeaders/wtf/Variant.h:390:35: warning: 'template<class _Tp> struct std::is_literal_type' is deprecated [-Wdeprecated-declarations] 390 | template<typename _Type,bool=std::is_literal_type<_Type>::value> | ^~~~~~~~~~~~~~~ This header is included in a lot of places, so it spams the build log pretty badly. Sadly, std::is_literal_type was removed without replacement. More info here: https://stackoverflow.com/questions/40351816/deprecated-stdis-literal-type-in-c17#40352351. So solution is a little unclear. I might just suppress the warning with IGNORE_WARNINGS and keep the current behavior... other proposals welcome.
<rdar://problem/73509470>
Maybe we can look for newer Variant implementations? We added WTF::Variant to get something like std::variant earlier. Like WTF::Optional, some of us intended this as a stopgap, and eventually we’d just move along to std::variant. But in the case of WTF::Optional, we grew to like at least one of the semantics we chose for our version, and may delay switching over indefinitely. It’s possible that is not try for WTF::Variant, so perhaps we can just switch over to std::variant?
(In reply to Darin Adler from comment #2) > It’s possible that is not try for WTF::Variant, so perhaps we can just switch over to std::variant? You're right. We should give this a try.
Here are my preferences for solutions to this: - switch to std::variant - update to (or maybe it’s "merge in") a newer version of the variant implementation; in r204227, Sam Weinig got it from https://bitbucket.org/anthonyw/variant/src and we could look there for a newer version - disable this warning just for Variant.h - patch our Variant.h to sidestep the lack of is_literal_type (might be straightforward to do it at least well enough to not break WebKit?) - disable this warning globally We can also do any combination of these, and do one first and then move up to the "better solutions" later. I’d like to climb from the bottom up to the top as far as we can go.
(In reply to Darin Adler from comment #4) > - disable this warning just for Variant.h I'm going to take the path of least resistance and disable the warning... but it only needs to be done for the one function where it's used, not for the entire file.
(No patch yet because I got sucked into a quixotic quest to see how hard it would be to switch to std::variant after all.)
It's not necessarily hard, but we use it in a lot of places, and it requires more time than I have available to adjust them all. I also attempted to hoist std::variant and its related methods (std::get, std::visit, std::holds_alternative) into the WTF namespace, but failed at that too. We also have one customization, WTF::switchOn, that could justify keeping wtf/Variant.h even if we were to successfully migrate to std::variant. Next I tried adjusting the code to no longer use std::is_literal_type, but after squinting for a while I decided I'd better not touch it. I tried searching for an updated version of our variant, but the upstream no longer exists. I tried searching for a new upstream that we could use. https://github.com/mpark/variant/ looks like a good option, and it's license-compatible. One compiler warning seems like not a very great reason to switch from one implementation to another, though. I'm just going to silence the warning. This function is removed from C++ 20, but I assume libstdc++ and libc++ will both keep it around forever. If it ever gets removed, then we'll need to revisit.
(In reply to Michael Catanzaro from comment #7) > I'm just going to silence the warning. Good call, thumbs up. I’m eager to *eventually* get to std::variant and also to std::optional and I hope someone will do those projects.
Created attachment 422138 [details] Patch
Comment on attachment 422138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422138&action=review > Source/WTF/ChangeLog:6607 > - use the RunLoop. Let's match them for consistency, and to delete some > + As of https://bugs.webkit.org/show_bug.cgi?id=213063, Darwin platforms > + use the RunLoop. Let's match them for consistency, and to delete some The old ChangeLog here got corrupted somehow, and gedit won't allow saving the original data. This is the sort of thing that's probably best fixed by pushing to SVN directly without using webkit-patch, but I don't have an SVN account anymore. I've just been relying on commit-queue since I left Igalia.
(In reply to Darin Adler from comment #8) > Good call, thumbs up. > > I’m eager to *eventually* get to std::variant and also to std::optional and > I hope someone will do those projects. I would like to use std::optional too, but I think WTF::Optional has some important subtle behavior difference that our code may rely on in an unknown number of places. But I cannot remember what it is! We had a long webkit-dev mailing list conversation about it a few years ago, but after searching our list archives for two minutes, I wasn't immediately able to find it. In contrast, switching to std::variant should *probably* be fairly safe.
Comment on attachment 422138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=422138&action=review >> Source/WTF/ChangeLog:6607 >> + use the RunLoop. Let's match them for consistency, and to delete some > > The old ChangeLog here got corrupted somehow, and gedit won't allow saving the original data. > > This is the sort of thing that's probably best fixed by pushing to SVN directly without using webkit-patch, but I don't have an SVN account anymore. I've just been relying on commit-queue since I left Igalia. Seems fine to do it this way. I don’t really understand the role of "gedit" here, but the patch seems fine.
gedit is the GNOME text editor!
WTF::Optional has a difference from some std::optional implementations: the move assignment operator sets the old object to WTF::nullopt. This difference is something we may have relied on in the past but we do not have solid evidence that we still rely on it; in fact when testing a patch to convert to std::optional I saw no test failures. In new code we try to use std::exchange when we rely on that behavior.
Committed r273841: <https://commits.webkit.org/r273841> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422138 [details].