236036 2022-02-02 12:44:28 -0800 [GLIB] Unreviewed debug buildfix after r288872 2022-02-02 16:49:49 -0800 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=235942 https://bugs.webkit.org/show_bug.cgi?id=235401 P2 Normal --- 1 lmoura lmoura aboxhall andresg_22 aperez apinheiro bugs-noreply cfleizach darin dmazzoni ews-watchlist jcraig jdiggs samuel_white oldest_to_newest 1836590 0 lmoura 2022-02-02 12:44:28 -0800 [GLIB] Unreviewed debug buildfix after r288872 1836591 1 450689 lmoura 2022-02-02 12:45:40 -0800 Created attachment 450689 [fast-cq] Patch 1836596 2 ews-feeder 2022-02-02 12:55:28 -0800 Committed r288990 (246709@main): <https://commits.webkit.org/246709@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 450689. 1836597 3 450689 darin 2022-02-02 12:57:20 -0800 Comment on attachment 450689 [fast-cq] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=450689&action=review > Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp:707 > - static constexpr SortedArrayMap roleNamesMap { roleNames }; > + static const SortedArrayMap roleNamesMap { roleNames }; This change looks like a bad idea. Could you explain why it wasn’t compiling? 1836606 4 lmoura 2022-02-02 13:14:31 -0800 (In reply to Darin Adler from comment #3) > Comment on attachment 450689 [details] > [fast-cq] Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=450689&action=review > > > Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp:707 > > - static constexpr SortedArrayMap roleNamesMap { roleNames }; > > + static const SortedArrayMap roleNamesMap { roleNames }; > > This change looks like a bad idea. Could you explain why it wasn’t compiling? This was the first bug that failed: https://build.webkit.org/#/builders/66/builds/7404 /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/SortedArrayMap.h: In static member function ‘static const char* WebCore::AccessibilityAtspi::localizedRoleName(WebCore::AccessibilityRole)’: /app/webkit/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp:705:62: in ‘constexpr’ expansion of ‘WTF::SortedArrayMap<std::pair<WebCore::AccessibilityRole, WebCore::RoleNameEntry> [122]>(WebCore::roleNames)’ /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/Assertions.h:364:34: error: call to non-‘constexpr’ function ‘void WTFReportAssertionFailure(const char*, int, const char*, const char*)’ 364 | WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \ | ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/SortedArrayMap.h:151:5: note: in expansion of macro ‘ASSERT_UNDER_CONSTEXPR_CONTEXT’ 151 | ASSERT_UNDER_CONSTEXPR_CONTEXT(isSortedConstExpr(std::begin(array), std::end(array), [] (auto& a, auto b) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/Assertions.h:212:25: note: ‘void WTFReportAssertionFailure(const char*, int, const char*, const char*)’ declared here 212 | WTF_EXPORT_PRIVATE void WTFReportAssertionFailure(const char* file, int line, const char* function, const char* assertion); | ^~~~~~~~~~~~~~~~~~~~~~~~~ 1836607 5 lmoura 2022-02-02 13:18:18 -0800 And the clang error: In file included from /app/webkit/WebKitBuild/Debug/WebCore/DerivedSources/unified-sources/UnifiedSource-aba958d6-7.cpp:4: /app/webkit/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp:707:37: error: constexpr variable 'roleNamesMap' must be initialized by a constant expression static constexpr SortedArrayMap roleNamesMap { roleNames }; ^~~~~~~~~~~~~~~~~~~~~~~~~~ /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/SortedArrayMap.h:151:5: note: non-constexpr function 'WTFReportAssertionFailure' cannot be used in a constant expression ASSERT_UNDER_CONSTEXPR_CONTEXT(isSortedConstExpr(std::begin(array), std::end(array), [] (auto& a, auto b) { ^ /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/Assertions.h:364:9: note: expanded from macro 'ASSERT_UNDER_CONSTEXPR_CONTEXT' WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, #assertion); \ ^ /app/webkit/Source/WebCore/accessibility/atspi/AccessibilityAtspi.cpp:707:37: note: in call to 'SortedArrayMap(roleNames)' static constexpr SortedArrayMap roleNamesMap { roleNames }; ^ /app/webkit/WebKitBuild/Debug/WTF/Headers/wtf/Assertions.h:212:25: note: declared here WTF_EXPORT_PRIVATE void WTFReportAssertionFailure(const char* file, int line, const char* function, const char* assertion); ^ 1836608 6 darin 2022-02-02 13:20:31 -0800 So ASSERT_UNDER_CONSTEXPR_CONTEXT is not working on this platform. It’s not working under constexpr context. We need to fix that. Not by changing this line of code, but but fixing the implementation of ASSERT_UNDER_CONSTEXPR_CONTEXT in the Assertions.h header. It’s working on newer versions of clang. We can’t keep dumbing down the rest of our code; even if we turned off those assertions entirely that would be better than un-constexpr'ing code whenever we run into this problem. 1836609 7 darin 2022-02-02 13:22:21 -0800 The fix might be to change "if" into "if constexpr". But whatever the fix is, the fix has to be a fix to make ASSERT_UNDER_CONSTEXPR_CONTEXT work. 1836631 8 lmoura 2022-02-02 14:05:36 -0800 It seems the failure happened because the array being passed to SortedArrayMap wasn't fully ordered, failing the isSortedConstExpr and thus generating the call to WTFReportAssertionFailure and the non-constexpr error. Example of a similar behavior: https://godbolt.org/z/6hzshsvdE In this specific case (build error), fixing the array order makes the build pass. I'll reopen and submit an updated patch reverting the original one. But indeed ASSERT_UNDER_CONSTEXPR_CONTEXT could provide a more meaningful error message in this config. 1836633 9 darin 2022-02-02 14:09:14 -0800 (In reply to Lauro Moura from comment #8) > It seems the failure happened because the array being passed to > SortedArrayMap wasn't fully ordered, failing the isSortedConstExpr and thus > generating the call to WTFReportAssertionFailure and the non-constexpr error. That makes total sense, great detective work! We have to fix that because it literally might not find some of the items in the map if it’s not sorted. So we definitely need to roll this out. That also means that the assertion will be hit at runtime after patching constexpr to be const. > But indeed ASSERT_UNDER_CONSTEXPR_CONTEXT could provide a more meaningful > error message in this config. I don’t know if that is practical. But we could at least name the WTFReportAssertionFailure function, or a function that is called here, with a clue about what the real problem might be (the assertion failed at compile time!). 1836634 10 darin 2022-02-02 14:10:03 -0800 (In reply to Darin Adler from comment #9) > So we definitely need to roll this out. Revert this constexpr->const change, I meant. 1836641 11 450698 lmoura 2022-02-02 14:42:32 -0800 Created attachment 450698 [fast-cq] Actual fix 1836647 12 aperez 2022-02-02 14:57:43 -0800 Nice found, and really cool that the compile-time assertion can signal that the input is not pre-sorted 🤯️ 1836707 13 ews-feeder 2022-02-02 16:49:44 -0800 Committed r289016 (246725@main): <https://commits.webkit.org/246725@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 450698. 450689 2022-02-02 12:45:40 -0800 2022-02-02 14:42:26 -0800 [fast-cq] Patch bug-236036-20220202204538.patch text/plain 1425 lmoura U3VidmVyc2lvbiBSZXZpc2lvbjogMjg4OTY5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDRiOThkODM1YTgzNjc5 NDk2ZmNhOGZjMDExMTMxYmEyNzRiZjM5NC4uMGQ3ZmYxNDhiODA2NWM3YzM0MmMyNTRjODAxYmY3 NDAwMTFjMGI1ZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDExIEBACisyMDIyLTAyLTAyICBMYXVy byBNb3VyYSAgPGxtb3VyYUBpZ2FsaWEuY29tPgorCisgICAgICAgIFtHTElCXSBVbnJldmlld2Vk IGRlYnVnIGJ1aWxkZml4IGFmdGVyIHIyODg4NzIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTIzNjAzNgorCisgICAgICAgICogYWNjZXNzaWJpbGl0eS9h dHNwaS9BY2Nlc3NpYmlsaXR5QXRzcGkuY3BwOgorICAgICAgICAoV2ViQ29yZTo6QWNjZXNzaWJp bGl0eUF0c3BpOjpsb2NhbGl6ZWRSb2xlTmFtZSk6CisKIDIwMjItMDItMDIgIEFudG9pbmUgUXVp bnQgIDxncmFvdXRzQHdlYmtpdC5vcmc+CiAKICAgICAgICAgQ1NTUHJvcGVydHlBbmltYXRpb246 OmFuaW1hdGFibGVTaG9ydGhhbmRzQWZmZWN0aW5nUHJvcGVydHkoKSBpcyB1bnVzZWQKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvYXRzcGkvQWNjZXNzaWJpbGl0eUF0 c3BpLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvYXRzcGkvQWNjZXNzaWJpbGl0 eUF0c3BpLmNwcAppbmRleCAzMTIzMjI1MTk0MWI1NWJjZjhhZTBlNTAxZmFkOWRjMWRlZmFhOWIx Li42Yzc3MmU2MmYxZDBiYWRiNTc5ODBmYjEwZmY3Nzg5ZjUyMjEzZDE2IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L2F0c3BpL0FjY2Vzc2liaWxpdHlBdHNwaS5jcHAK KysrIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9hdHNwaS9BY2Nlc3NpYmlsaXR5QXRz cGkuY3BwCkBAIC03MDQsNyArNzA0LDcgQEAgc3RhdGljIGNvbnN0ZXhwciBzdGQ6OnBhaXI8QWNj ZXNzaWJpbGl0eVJvbGUsIFJvbGVOYW1lRW50cnk+IHJvbGVOYW1lc1tdID0gewogCiBjb25zdCBj aGFyKiBBY2Nlc3NpYmlsaXR5QXRzcGk6OmxvY2FsaXplZFJvbGVOYW1lKEFjY2Vzc2liaWxpdHlS b2xlIHJvbGUpCiB7Ci0gICAgc3RhdGljIGNvbnN0ZXhwciBTb3J0ZWRBcnJheU1hcCByb2xlTmFt ZXNNYXAgeyByb2xlTmFtZXMgfTsKKyAgICBzdGF0aWMgY29uc3QgU29ydGVkQXJyYXlNYXAgcm9s ZU5hbWVzTWFwIHsgcm9sZU5hbWVzIH07CiAgICAgaWYgKGF1dG8gZW50cnkgPSByb2xlTmFtZXNN YXAudHJ5R2V0KHJvbGUpKQogICAgICAgICByZXR1cm4gZW50cnktPmxvY2FsaXplZE5hbWU7CiAK 450698 2022-02-02 14:42:32 -0800 2022-02-02 16:49:47 -0800 [fast-cq] Actual fix bug-236036-20220202224230.patch text/plain 2689 lmoura U3VidmVyc2lvbiBSZXZpc2lvbjogMjg4OTk1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTU5YjJjYWIxYzRmZDc0 MGMyMmYyYWY2YTczY2E1YjdiOWMyOTBlYS4uYjg3OTM2MzM3MTJkODI0OWU0MmE4ZmJlNWI4NWI5 MzQwMDFkNDhmMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDIyLTAyLTAyICBMYXVy byBNb3VyYSAgPGxtb3VyYUBpZ2FsaWEuY29tPgorCisgICAgICAgIFtHTElCXSBVbnJldmlld2Vk IGRlYnVnIGJ1aWxkZml4IGFmdGVyIHIyODg4NzIKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTIzNjAzNgorCisgICAgICAgIFRoZSBhY3R1YWwgaXNzdWUg d2FzIHRoZSByb2xlTmFtZXMgYXJyYXkgcHJvdmlkZWQgdG8gU29ydGVkQXJyYXlNYXAKKyAgICAg ICAgY29uc3RydWN0b3Igd2Fzbid0IGZ1bGx5IG9yZGVyZWQsIG1ha2luZyBpc1NvcnRlZENvbnN0 RXhwciB0byBmYWlsLAorICAgICAgICBnZW5lcmF0aW5nIHRoZSBjYWxsIHRvIFdURlJlcG9ydC4u Liwgd2hpY2ggaXMgbm9uLWNvbnN0ZXhwciBhbmQgdGh1cworICAgICAgICB0aGUgY29tcGlsZSBm YWlsdXJlLgorCisgICAgICAgIFRoaXMgY29tbWl0IHJldmVydCB0aGUgcHJldmlvdXMgZml4IGZy b20gcjI4ODk5MCBhbmQgZml4IHRoZSBvcmRlcmluZyBvZgorICAgICAgICByb2xlTmFtZXMuCisK KyAgICAgICAgKiBhY2Nlc3NpYmlsaXR5L2F0c3BpL0FjY2Vzc2liaWxpdHlBdHNwaS5jcHA6Cisg ICAgICAgIChXZWJDb3JlOjpBY2Nlc3NpYmlsaXR5QXRzcGk6OmxvY2FsaXplZFJvbGVOYW1lKToK KwogMjAyMi0wMi0wMiAgU2FpZCBBYm91LUhhbGxhd2EgIDxzYWlkQGFwcGxlLmNvbT4KIAogICAg ICAgICBbR1BVIFByb2Nlc3NdIFJlbW92ZSBEaXNwbGF5TGlzdDo6UmVwbGF5ZXI6OkRlbGVnYXRl IGFuZCBEaXNwbGF5TGlzdDo6UmVjb3JkZXJJbXBsOjpEZWxlZ2F0ZQpkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9hdHNwaS9BY2Nlc3NpYmlsaXR5QXRzcGkuY3BwIGIv U291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9hdHNwaS9BY2Nlc3NpYmlsaXR5QXRzcGkuY3Bw CmluZGV4IDZjNzcyZTYyZjFkMGJhZGI1Nzk4MGZiMTBmZjc3ODlmNTIyMTNkMTYuLjMwMWEzZTIw MTJlOTJiNzY5ODE4NDQxNWJjZDIxMjkzNTIyODExYTUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2FjY2Vzc2liaWxpdHkvYXRzcGkvQWNjZXNzaWJpbGl0eUF0c3BpLmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L2F0c3BpL0FjY2Vzc2liaWxpdHlBdHNwaS5jcHAKQEAg LTYwMiw4ICs2MDIsOCBAQCBzdGF0aWMgY29uc3RleHByIHN0ZDo6cGFpcjxBY2Nlc3NpYmlsaXR5 Um9sZSwgUm9sZU5hbWVFbnRyeT4gcm9sZU5hbWVzW10gPSB7CiAgICAgeyBBY2Nlc3NpYmlsaXR5 Um9sZTo6RGVmaW5pdGlvbiwgeyAiZGVmaW5pdGlvbiIsIE5fKCJkZWZpbml0aW9uIikgfSB9LAog ICAgIHsgQWNjZXNzaWJpbGl0eVJvbGU6OkRlbGV0aW9uLCB7ICJjb250ZW50IGRlbGV0aW9uIiwg Tl8oImNvbnRlbnQgZGVsZXRpb24iKSB9IH0sCiAgICAgeyBBY2Nlc3NpYmlsaXR5Um9sZTo6RGVz Y3JpcHRpb25MaXN0LCB7ICJkZXNjcmlwdGlvbiBsaXN0IiwgTl8oImRlc2NyaXB0aW9uIGxpc3Qi KSB9IH0sCi0gICAgeyBBY2Nlc3NpYmlsaXR5Um9sZTo6RGVzY3JpcHRpb25MaXN0VGVybSwgeyAi ZGVzY3JpcHRpb24gdGVybSIsIE5fKCJkZXNjcmlwdGlvbiB0ZXJtIikgfSB9LAogICAgIHsgQWNj ZXNzaWJpbGl0eVJvbGU6OkRlc2NyaXB0aW9uTGlzdERldGFpbCwgeyAiZGVzY3JpcHRpb24gdmFs dWUiLCBOXygiZGVzY3JpcHRpb24gdmFsdWUiKSB9IH0sCisgICAgeyBBY2Nlc3NpYmlsaXR5Um9s ZTo6RGVzY3JpcHRpb25MaXN0VGVybSwgeyAiZGVzY3JpcHRpb24gdGVybSIsIE5fKCJkZXNjcmlw dGlvbiB0ZXJtIikgfSB9LAogICAgIHsgQWNjZXNzaWJpbGl0eVJvbGU6OkRpcmVjdG9yeSwgeyAi ZGlyZWN0b3J5IHBhbmUiLCBOXygiZGlyZWN0b3J5IHBhbmUiKSB9IH0sCiAgICAgeyBBY2Nlc3Np YmlsaXR5Um9sZTo6RGl2LCB7ICJzZWN0aW9uIiwgTl8oInNlY3Rpb24iKSB9IH0sCiAgICAgeyBB Y2Nlc3NpYmlsaXR5Um9sZTo6RG9jdW1lbnQsIHsgImRvY3VtZW50IGZyYW1lIiwgTl8oImRvY3Vt ZW50IGZyYW1lIikgfSB9LApAQCAtNzA0LDcgKzcwNCw3IEBAIHN0YXRpYyBjb25zdGV4cHIgc3Rk OjpwYWlyPEFjY2Vzc2liaWxpdHlSb2xlLCBSb2xlTmFtZUVudHJ5PiByb2xlTmFtZXNbXSA9IHsK IAogY29uc3QgY2hhciogQWNjZXNzaWJpbGl0eUF0c3BpOjpsb2NhbGl6ZWRSb2xlTmFtZShBY2Nl c3NpYmlsaXR5Um9sZSByb2xlKQogewotICAgIHN0YXRpYyBjb25zdCBTb3J0ZWRBcnJheU1hcCBy b2xlTmFtZXNNYXAgeyByb2xlTmFtZXMgfTsKKyAgICBzdGF0aWMgY29uc3RleHByIFNvcnRlZEFy cmF5TWFwIHJvbGVOYW1lc01hcCB7IHJvbGVOYW1lcyB9OwogICAgIGlmIChhdXRvIGVudHJ5ID0g cm9sZU5hbWVzTWFwLnRyeUdldChyb2xlKSkKICAgICAgICAgcmV0dXJuIGVudHJ5LT5sb2NhbGl6 ZWROYW1lOwogCg==