220662 2021-01-15 11:52:00 -0800 std::is_literal_type causes -Wdeprecated-declarations warning with GCC 11 2021-03-03 14:23:24 -0800 1 1 1 Unclassified WebKit Web Template Framework WebKit Nightly Build PC Linux RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=211674 InRadar P2 Normal --- 1 mcatanzaro mcatanzaro benjamin cdumez cmarcelo darin ews-watchlist mcatanzaro sam simon.fraser webkit-bug-importer oldest_to_newest 1719936 0 mcatanzaro 2021-01-15 11:52:00 -0800 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. 1721662 1 webkit-bug-importer 2021-01-22 11:52:15 -0800 <rdar://problem/73509470> 1721687 2 darin 2021-01-22 12:48:28 -0800 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? 1721700 3 mcatanzaro 2021-01-22 13:20:41 -0800 (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. 1721737 4 darin 2021-01-22 15:22:00 -0800 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. 1734958 5 mcatanzaro 2021-03-02 12:01:53 -0800 (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. 1735138 6 mcatanzaro 2021-03-02 17:20:52 -0800 (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.) 1735449 7 mcatanzaro 2021-03-03 12:43:55 -0800 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. 1735458 8 darin 2021-03-03 12:56:29 -0800 (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. 1735459 9 422138 mcatanzaro 2021-03-03 12:57:26 -0800 Created attachment 422138 Patch 1735460 10 422138 mcatanzaro 2021-03-03 12:58:40 -0800 Comment on attachment 422138 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. 1735463 11 mcatanzaro 2021-03-03 13:03:30 -0800 (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. 1735466 12 422138 darin 2021-03-03 13:04:52 -0800 Comment on attachment 422138 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. 1735470 13 mcatanzaro 2021-03-03 13:06:00 -0800 gedit is the GNOME text editor! 1735472 14 darin 2021-03-03 13:08:17 -0800 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. 1735520 15 ews-feeder 2021-03-03 14:23:21 -0800 Committed r273841: <https://commits.webkit.org/r273841> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422138. 422138 2021-03-03 12:57:26 -0800 2021-03-03 14:23:22 -0800 Patch bug-220662-20210303145726.patch text/plain 2049 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjczODI5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGZlYjU0ZTkwN2EwNTQ3MjFiMDE0MmMz MjY1NDVlMWQ2NGYyNmRkZTQuLmQyMTgzNDBjMDA0ZDJiYzQ5OTI2ZmU4OGJmNjdmYWJhOTIzNWY2 ZjAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMjEtMDMtMDMgIE1pY2hhZWwgQ2F0YW56YXJvICA8 bWNhdGFuemFyb0Bnbm9tZS5vcmc+CisKKyAgICAgICAgc3RkOjppc19saXRlcmFsX3R5cGUgY2F1 c2VzIC1XZGVwcmVjYXRlZC1kZWNsYXJhdGlvbnMgd2FybmluZyB3aXRoIEdDQyAxMQorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjIwNjYyCisgICAgICAg IDxyZGFyOi8vcHJvYmxlbS83MzUwOTQ3MD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkg KE9PUFMhKS4KKworICAgICAgICBJZ25vcmUgdGhlIHdhcm5pbmcuIEl0IHdvdWxkIGJlIGJldHRl ciB0byBub3QgdXNlIHRoZSBkZXByZWNhdGVkIHN0ZDo6aXNfbGl0ZXJhbF90eXBlLCBidXQKKyAg ICAgICAgdGhpcyB3b3JrcyBmb3Igbm93LgorCisgICAgICAgICogd3RmL1ZhcmlhbnQuaDoKKwog MjAyMS0wMy0wMyAgWW91ZW5uIEZhYmxldCAgPHlvdWVubkBhcHBsZS5jb20+CiAKICAgICAgICAg V2ViS2l0TGVnYWN5IG5lZWRzIHRvIGtlZXAgSlNET01XaW5kb3cgZXZlbiB0aG91Z2ggaXQgaXMg dXNlZCB3aGlsZSBpdHMgb3JpZ2luIGlzIG5vdCBzZXQKQEAgLTY1OTAsNyArNjYwMyw4IEBACiAK ICAgICAgICAgUmV2aWV3ZWQgYnkgQ2FybG9zIEdhcmNpYSBDYW1wb3MuCiAKLSAgICAgAAAAAAAA AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA AAAAAAAAAAAAAAAgICAgICAgIHVzZSB0aGUgUnVuTG9vcC4gTGV0J3MgbWF0Y2ggdGhlbSBmb3Ig Y29uc2lzdGVuY3ksIGFuZCB0byBkZWxldGUgc29tZQorICAgICAgICBBcyBvZiBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEzMDYzLCBEYXJ3aW4gcGxhdGZvcm1zCisg ICAgICAgIHVzZSB0aGUgUnVuTG9vcC4gTGV0J3MgbWF0Y2ggdGhlbSBmb3IgY29uc2lzdGVuY3ks IGFuZCB0byBkZWxldGUgc29tZQogICAgICAgICBjb2RlLgogCiAgICAgICAgICogd3RmL2dlbmVy aWMvTWFpblRocmVhZEdlbmVyaWMuY3BwOgpkaWZmIC0tZ2l0IGEvU291cmNlL1dURi93dGYvVmFy aWFudC5oIGIvU291cmNlL1dURi93dGYvVmFyaWFudC5oCmluZGV4IDFhODk0MDU4ZDk1M2U1NjA1 MzE4Y2MyZmQ0NDAzYzY3NTRhN2JjYTguLjNjYTdlYmZlOWQwNDc5NmQwYjhhMDU2NzNjY2EwZmUz MThlMDJkMzIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL1ZhcmlhbnQuaAorKysgYi9Tb3Vy Y2UvV1RGL3d0Zi9WYXJpYW50LmgKQEAgLTM4Nyw2ICszODcsOCBAQCBzdHJ1Y3QgX19hbnlfYmFj a3VwX3N0b3JhZ2VfcmVxdWlyZWQ8VmFyaWFudDxfVHlwZXMuLi4+ID57CiB0ZW1wbGF0ZTx0eXBl bmFtZSAuLi4gX1R5cGVzPgogdW5pb24gX192YXJpYW50X2RhdGE7CiAKKy8vIHN0ZDo6aXNfbGl0 ZXJhbF90eXBlIGlzIGRlcHJlY2F0ZWQgaW4gQysrMTcgYW5kIHJlbW92ZWQgaW4gQysrMjAKK0FM TE9XX0RFUFJFQ0FURURfREVDTEFSQVRJT05TX0JFR0lOCiB0ZW1wbGF0ZTx0eXBlbmFtZSBfVHlw ZSxib29sPXN0ZDo6aXNfbGl0ZXJhbF90eXBlPF9UeXBlPjo6dmFsdWU+CiBzdHJ1Y3QgX192YXJp YW50X3N0b3JhZ2V7CiAgICAgdHlwZWRlZiBfVHlwZSBfX3R5cGU7CkBAIC00MDUsNiArNDA3LDcg QEAgc3RydWN0IF9fdmFyaWFudF9zdG9yYWdlewogICAgIH0KICAgICBzdGF0aWMgdm9pZCBfX2Rl c3Ryb3koX190eXBlJil7fQogfTsKK0FMTE9XX0RFUFJFQ0FURURfREVDTEFSQVRJT05TX0VORAog CiB0ZW1wbGF0ZTx0eXBlbmFtZSBfVHlwZT4KIHN0cnVjdCBfX3N0b3JhZ2Vfd3JhcHBlcnsK