S2 Master Fails to build with clang14

[crroush]

There are numerous errors when building with g++12.3.0 in Ubuntu 22.04 off master [035272b]

The following errors repeat a bunch of times. Building using g++11.4 works fine.

2-src/src/s2/base/port.h:35:3: error: no matching function for call to 'operator delete'
  ::operator delete(ptr, size);
  ^~~~~~~~~~~~~~~~~
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/new:180:13: note: candidate function not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'void *' for 2nd argument; take the address of the argument with &
inline void operator delete  (void*, void*) _GLIBCXX_USE_NOEXCEPT { }
            ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/new:144:6: note: candidate function not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'const std::nothrow_t' for 2nd argument
void operator delete(void*, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT
     ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/new:153:6: note: candidate function not viable: no known conversion from 'size_t' (aka 'unsigned long') to 'std::align_val_t' for 2nd argument
void operator delete(void*, std::align_val_t)
     ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/new:130:6: note: candidate function not viable: requires 1 argument, but 2 were provided
void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
     ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/new:155:6: note: candidate function not viable: requires 3 arguments, but 2 were provided
void operator delete(void*, std::align_val_t, const std::nothrow_t&)
     ^

[jmr]

Try adding #include <new>.

Never mind, that's not it.

[jmr]

It works on godbolt: https://godbolt.org/z/Ta4dj9d8x

Can you run cmake with -DCMAKE_VERBOSE_MAKEFILE=1, then show the compilation command, and the g++ --version output from the g++ that actually gets used?

Do you have a line like this new your <new>?

https://github.com/gcc-mirror/gcc/blob/90a791bb651d38d35f639d34880792a1d4aea3cb/libstdc%2B%2B-v3/libsupc%2B%2B/new#L135

Is __cpp_sized_deallocation true?

[jmr]

I added ubuntu-22 to the workflow and it passes, but it uses gcc 11.

https://github.com/google/s2geometry/actions/runs/13604057711/job/38033461524?pr=412#step:6:10

ubuntu-24 uses gcc 13.

I can try to use a more comprehensive matrix of test setups.

[crroush]

@jmr just getting back in the game here.
So for g++12

g++ (Ubuntu 12.3.0-1ubuntu1~22.04) 12.3.0
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

in /usr/include/c++12/new I have the exact line you linked above

I will need to see if __cpp_size_deallocation is set to true

I need to move S2 to be in it's own build environment than in my project before I can get you the verbose output. But I suspect if you add to your workflow installing g++12.3 you should hopefully be able to replicate my issue.

[jmr]

I need to move S2 to be in it's own build environment than in my project before I can get you the verbose output.

Did you have a chance to do this?

If you're building it as part of your project, what C++ standard are you using?

[crroush]

I finally got back to look at this, so when I stripped out my build environment, it turns out I was building with clang 14
There were parts of that project that dropped to g++12 so it was a bit confusing when I looked at the logs.

That said I put together a simple project to build just absl master and s2 master

out.txt

[jmr]

it turns out I was building with clang 14

Ok, at least things make a bit of sense now. I can reproduce that this doesn't work with clang 14 on godbolt: https://godbolt.org/z/xKKEzYzEd

It works if you add -fsized-deallocation, so that's a work-around: https://godbolt.org/z/aon15MTvo

I think this should always be enabled for C++14, but I'm not sure.

It starts working without -fsized-deallocation in clang 19: https://godbolt.org/z/osWcGveso

I will add this to the flags since we want to support clang >= 7.

https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md

[jmr]

Can you try adding add_compile_options(-fsized-deallocation) after here?

s2geometry/CMakeLists.txt

Line 86 in 58de4ea

add_definitions(-Wno-deprecated-declarations)

I'm not sure why the other ones use add_definitions().

[crroush]

That fixed it for me.
Thanks for digging into this.
Here is the output.
out.txt