serenity icon indicating copy to clipboard operation
serenity copied to clipboard

AK: Remove workarounds for missing P0960R3 support in Clang

Open DanShaders opened this issue 1 year ago • 7 comments

DanShaders avatar May 17 '24 02:05 DanShaders

The all red CI tells me that in fact, P0690R3 support in clang is not without bugs :D

ADKaster avatar May 17 '24 03:05 ADKaster

Have the oss-fuzz images been updated now? (ref: https://github.com/SerenityOS/serenity/pull/22264)

stelar7 avatar May 17 '24 10:05 stelar7

Should be, oss-fuzz is currently at Clang 17+\varepsilon version.

DanShaders avatar May 17 '24 10:05 DanShaders

Have you checked if the latest Xcode can cope with this?

BertalanD avatar May 18 '24 17:05 BertalanD

No, I don't have access to MacOS

DanShaders avatar May 18 '24 17:05 DanShaders

I patched this in and ran Meta/serenity.sh build lagom image on a mac with Xcode 15.2. That's the newest version that runs on macOS 13.5+ (I haven't installed macOS 14 yet), but not the very newest version (which is 15.4).

It failed like so:

[153/283] Building CXX object Userland/Libraries/LibGfx/CMakeFiles/LibGfx.dir/ImageFormats/TinyVGLoader.cpp.o
FAILED: Userland/Libraries/LibGfx/CMakeFiles/LibGfx.dir/ImageFormats/TinyVGLoader.cpp.o 
/Users/thakis/Downloads/ccache-4.9-darwin/ccache /usr/bin/clang++ -DENABLE_COMPILETIME_FORMAT_CHECK -DLibGfx_EXPORTS -I/Users/thakis/src/serenity/Meta/Lagom/../.. -I/Users/thakis/src/serenity/Meta/Lagom/../../Userland -I/Users/thakis/src/serenity/Meta/Lagom/../../Userland/Libraries -I/Users/thakis/src/serenity/Meta/Lagom/../../Userland/Services -I/Users/thakis/src/serenity/Build/lagom -I/Users/thakis/src/serenity/Build/lagom/Userland/Libraries -I/Users/thakis/src/serenity/Build/lagom/Userland/Services -arch arm64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.2.sdk -mmacosx-version-min=13.5 -fPIC -fcolor-diagnostics -Wall -Wextra -Wno-invalid-offsetof -Wno-unknown-warning-option -Wno-unused-command-line-argument -fno-exceptions -ffp-contract=off -Werror -fconstexpr-steps=16777216 -Wno-implicit-const-int-float-conversion -Wno-user-defined-literals -Wno-vla-cxx-extension -Wno-maybe-uninitialized -Wno-shorten-64-to-32 -fsigned-char -ggnu-pubnames -fPIC -O2 -g1 -Wno-overloaded-virtual -Wno-unused-private-field -std=c++2b -MD -MT Userland/Libraries/LibGfx/CMakeFiles/LibGfx.dir/ImageFormats/TinyVGLoader.cpp.o -MF Userland/Libraries/LibGfx/CMakeFiles/LibGfx.dir/ImageFormats/TinyVGLoader.cpp.o.d -o Userland/Libraries/LibGfx/CMakeFiles/LibGfx.dir/ImageFormats/TinyVGLoader.cpp.o -c /Users/thakis/src/serenity/Userland/Libraries/LibGfx/ImageFormats/TinyVGLoader.cpp
/Users/thakis/src/serenity/Userland/Libraries/LibGfx/ImageFormats/TinyVGLoader.cpp:549:19: error: no matching function for call to 'make'
    : m_context { make<TinyVGLoadingContext>(FixedMemoryStream { bytes }) }
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/thakis/src/serenity/Meta/Lagom/../../AK/NonnullOwnPtr.h:148:63: note: candidate template ignored: constraints not satisfied [with T = Gfx::TinyVGLoadingContext, Args = <AK::FixedMemoryStream>]
requires(IsConstructible<T, Args...>) inline NonnullOwnPtr<T> make(Args&&... args)
                                                              ^
/Users/thakis/src/serenity/Meta/Lagom/../../AK/NonnullOwnPtr.h:148:10: note: because 'IsConstructible<Gfx::TinyVGLoadingContext, AK::FixedMemoryStream>' evaluated to false
requires(IsConstructible<T, Args...>) inline NonnullOwnPtr<T> make(Args&&... args)
         ^
1 error generated.

nico avatar May 18 '24 17:05 nico

Which llvm version is your Apple Clang based on?

Edit: oh, cppreference says that P0960 isn't supported on Apple Clang at all. Let's then ifdef out these definition on everything except Apple Clang. (I really want to delete these because without them make gives an error inline in clangd)

DanShaders avatar May 18 '24 17:05 DanShaders

I really want to delete these because without them make gives an error inline in clangd

Do you mean that the currently present workaround causes an issue with your clangd config? In that case, let's merge this. Otherwise, I don't see a point in #ifdef-ing out the code right now.

BertalanD avatar May 21 '24 08:05 BertalanD

It doesn't really work around any issues, just makes life easier, i. e.:

Before: image + some long error when compiling

After: image

DanShaders avatar May 21 '24 12:05 DanShaders