Tests fail to build about outcome HOT 36 CLOSED

Tests are built successfully with rev. 1407d02

from outcome.

I added a FreeBSD CI runner to Outcome, and everything compiles and all tests pass: https://github.com/ned14/outcome/actions/runs/5868214302/job/15910531563

I think the problem is your side of things?

from outcome.

It's puzzling, I can't see what is wrong.

from outcome.

Your clang is trying to new(addr) void which shouldn't be possible, because void gets stripped out by devoid<T>. This is devoid<T>:

  struct void_type
  {
    // We always compare true to another instance of me
    constexpr bool operator==(void_type /*unused*/) const noexcept { return true; }
    constexpr bool operator!=(void_type /*unused*/) const noexcept { return false; }
  };
  template <class T> using devoid = std::conditional_t<std::is_void<T>::value, void_type, T>;

So, somehow this isn't working on your clang. I would suspect the std::is_void is broken somehow.

from outcome.

(Just to explain, clangs up to about 14 had good QoI, clangs 14 and after have become a hot mess of bug especially around type traits. In my last job we abandoned clang support entirely from it, and I'm getting very close to refusing to support clangs after 13 in all my open source until the clang maintainers fix their compiler)

from outcome.

What clang version is used in the tests?

from outcome.

Whatever is the default with FreeBSD 13. I assume that's clang 11, which is the last reliable good clang. clang 12 began the descent into hot bug mess, clang 12 doesn't compile some correct code and it's got continually worse with every clang release since.

Yesterday I switched LLFIO's CI to use Ubuntu 22.04 for Linux, and its default clang still chokes on the default libstdc++. Nobody in Ubuntu LTS maintenance fixed it upstream, which is just awful.

from outcome.

I don't use templates intensely so I didn't notice this.

from outcome.

Since Google and Apple dropped most resourcing of clang things have been going steadily downhill for that compiler. MSVC is actually more reliable than clang nowadays for C++ 20, which says a very great deal. Google pulled most of its resourcing after WG21 refused to ABI break. Apple pulled most of its resourcing somewhat earlier once Swift was mature for new development. Intel throws a bit into clang, but its lead clang devs have been leaving recently due to a pay freeze. If Aaron (head of clang at Intel) decides to also move on from Intel, I don't know what remains for clang personally.

It's only going to get worse, clang has ever fewer full time devs keeping it going with time. GCC and MSVC are likely to become the only production quality C++ 23 compilers, assuming IBM doesn't purge their GCC full time devs in the name of cost efficiencies.

I don't know what all this means for FreeBSD long term. You can either stick to an older clang which actually works, direct scarce FreeBSD resources into fixing clang, or return to using GCC as the primary compiler. None of those options are palatable for BSD in my opinion. Sorry.

from outcome.

@ned14
Do you know what would be a testcase for the std::is_void malfunction?
I tries it with many types and it behaves correctly.

from outcome.

cmake -G Ninja .. -DCMAKE_BUILD_TYPE=Debug && ninja test passes on universe13a, which is running FreeBSD 13.2 (userspace; kernel is 14.0). So this sounds like a local issue?

I'll note that devoid isn't relevant here, though, unless I'm mistaken. The issue is the placement new thinking that the memory you're trying to use is of type void rather than T *, i.e. the OUTCOME_ADDRESS_OF(_value) being of type void. Given that that's either (&_value) or (std::addressof(_value)) that makes no sense. I'll also note that OUTCOME_USE_STD_ADDRESSOF is defined to 1 solely for test/tests/core-result.cpp, which is what all the errors you're seeing are for. So it sounds to me like your local environment has an utterly-borked std::addressof?

FWIW:

[jrtc27@universe13a ~/outcome/build]$ uname -a
FreeBSD universe13a.freebsd.org 13.2-STABLE FreeBSD 14.0-CURRENT #0 main-n263287-2071e54c226a: Wed May 31 00:10:40 UTC 2023 [email protected]:/usr/obj/usr/src/sys/CLUSTER14 amd64
[jrtc27@universe13a ~/outcome/build]$ c++ --version
FreeBSD clang version 15.0.7 (https://github.com/llvm/llvm-project.git llvmorg-15.0.7-0-g8dfdcc7b7bf6)
Target: x86_64-unknown-freebsd13.2
Thread model: posix
InstalledDir: /usr/bin

from outcome.

Do you know what would be a testcase for the std::addressof malfunction?

from outcome.

That's a good point - this is only occurring on @yurivict's build system when std::addressof() is actually used.

It's hard to imagine how the implementation of that function could be borked UNLESS something is overriding it with an overly accepting overload set. So, is there anything else being included before Outcome includes its stuff?

(I suggest dumping the preprocessor output, search it for addressof)

from outcome.

I mean, does:

#include <memory>
int x;
int *p = std::addressof(x);

even work on your system? I have no clue how your system is broken, other than to say it is not the same as seen on the FreeBSD cluster.

Have you tried doing a build inside Poudriere rather than /usr/ports with all your own system's pollution? That'd isolate it to something in ports messing with it vs something with your system configuration specifically.

from outcome.

This code:

int main() {
        int x;
        int *p1 = std::addressof(x);
        int *p2 = &x;

        std::cout << "p1=" << p1 << std::endl;
        std::cout << "p2=" << p2 << std::endl;
}       

prints same addresses for p1 and p2.

Have you tried doing a build inside Poudriere rather than /usr/ports with all your own system's pollution?

I do this for every port I work on.
The issue with tests, however, seems to be related to the compiler.

My compiler is clang-15.0.7

from outcome.

You could try including all the headers which value_storage.hpp does before your test above. Obviously with OUTCOME_ADDRESS_OF() using std::addressof. Something is getting dragged in there which breaks std::addressof, or maybe, OUTCOME_ADDRESS_OF().

from outcome.

This code:

int main() {
        int x;
        int *p1 = std::addressof(x);
        int *p2 = &x;

        std::cout << "p1=" << p1 << std::endl;
        std::cout << "p2=" << p2 << std::endl;
}       

prints same addresses for p1 and p2.

Ok, so it's more complicated. As @ned14 says, the only real way to figure out what on earth is going on is to look at the preprocessed output for that specific test file, i.e. run:

/usr/bin/c++  -I/usr/ports/devel/outcome/work/outcome-2.2.7/include -isystem /usr/local/include -O2 -pipe -fstack-protector-strong -fno-strict-aliasing -O2 -pipe -fstack-protector-strong -fno-strict-aliasing   -DNDEBUG -fexceptions -frtti -fstack-protector-strong -Winvalid-pch -Xclang -include-pch -Xclang /usr/ports/devel/outcome/work/.build/CMakeFiles/outcome-dependency-smoke-test_1.dir/cmake_pch.hxx.pch -Xclang -include -Xclang /usr/ports/devel/outcome/work/.build/CMakeFiles/outcome-dependency-smoke-test_1.dir/cmake_pch.hxx -o CMakeFiles/outcome-dependency-smoke-test_1.dir/test/tests/core-result.cpp.i -E /usr/ports/devel/outcome/work/outcome-2.2.7/test/tests/core-result.cpp

and upload core-result.cpp.i somewhere.

My compiler is clang-15.0.7

So was I, and it worked just fine. So the compiler version isn't sufficient, nor is it necessarily necessary.

from outcome.

Here is core-result.cpp.i from my FreeBSD 13.2 system.

from outcome.

Why on earth is your clang implementing std::addressof() as a builtin? I see no gain, its default implementation is trivial C++.

template <class _Tp>
inline
__attribute__((__no_sanitize__("cfi"))) __attribute__((__visibility__("hidden"))) __attribute__((__exclude_from_explicit_instantiation__)) __attribute__((__abi_tag__("v15007")))
_Tp*
addressof(_Tp& __x) noexcept
{
    return __builtin_addressof(__x);
}
# 71 "/usr/include/c++/v1/__memory/addressof.h" 3
template <class _Tp> _Tp* addressof(const _Tp&&) noexcept = delete;

Your preprocessed output appears to not be defining OUTCOME_USE_STD_ADDRESSOF=1?

Have you tried disabling preprocessed headers? Outcome's CMakeLists.txt explicitly disables preprocessed headers for that TU.

from outcome.

Why on earth is your clang implementing std::addressof() as a builtin? I see no gain, its default implementation is trivial C++.

Same reason you switched from using & to std::addressof() optionally: because operator& can be overloaded.

Your preprocessed output appears to not be defining OUTCOME_USE_STD_ADDRESSOF=1?

Yeah, that's the problem here. You can see in fact in the command line that preprocessed headers are being used. And the instantiations that are being choked on are the ones where outcome tests having void operator&(), so are to be expected if you use & not std::addressof. So now the question is why CMake decided to do that.

from outcome.

Have you tried disabling preprocessed headers? Outcome's CMakeLists.txt explicitly disables preprocessed headers for that TU.

This uses outcome-2.2.7 sources verbatim. No patches are applied.
What exactly do I alter to disable preprocessed headers project-wide?

from outcome.

Same reason you switched from using & to std::addressof() optionally: because operator& can be overloaded.

Anybody who defines an overload for taking the address of a char deserves everything they get.

Equally, library authors must try to not get in the way of user mischief, no matter how insane. So a builtin I suppose makes sense.

Yeah, that's the problem here. You can see in fact in the command line that preprocessed headers are being used.

It is very important that preprocessed headers are not used for core-result.cpp, as it defines OUTCOME_USE_STD_ADDRESSOF=1 right at its top. That would not interact well with a preprocessed header for obvious reasons.

So now the question is why CMake decided to do that.

Obviously cmake is doing what it's told on systems which are not @yurivict's.

Here are the lines which disable precompiled headers for core-result.cpp:

• https://github.com/ned14/outcome/blob/develop/CMakeLists.txt#L188
• https://github.com/ned14/outcome/blob/develop/CMakeLists.txt#L221
• https://github.com/ned14/outcome/blob/develop/CMakeLists.txt#L252

EXCEPT here's the thing: it's been omitted from the first set by accident.

@yurivict Try adding outcome_hl--core-result to that first list and see if it fixes your problem.

from outcome.

Try adding outcome_hl--core-result to that first list and see if it fixes your problem.

The dependency smoke test is defined way to late for that, aint it?

from outcome.

I've stopped precompiled headers being applied to that test in develop branch.

from outcome.

@ned14 Looking at his logs I believe its the dependency smoke tests that are causing trouble and they won't profit from that exclusion. See the line from his failure log:

FAILED: CMakeFiles/outcome-dependency-smoke-test_1.dir/test/tests/core-result.cpp.o 

So we need to do something like

diff --git a/CMakeLists.txt b/CMakeLists.txt
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -367,8 +367,9 @@
   foreach(target ${OUTCOME_SMOKE_TESTS})
     target_link_libraries(${target} PRIVATE outcome::hl)
     set_target_properties(${target} PROPERTIES
       RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
+      DISABLE_PRECOMPILE_HEADERS On
     )
     add_test(NAME ${target} CONFIGURATIONS Debug Release RelWithDebInfo MinSizeRel
       COMMAND $<TARGET_FILE:${target}> --reporter junit --out $<TARGET_FILE:${target}>.junit.xml
     )

from outcome.

Try adding outcome_hl--core-result to that first list and see if it fixes your problem.

Nothing changed.

from outcome.

Fixed as per your suggestion.

from outcome.

@yurivict would you mind trying my patch?

from outcome.

Ah, yes, the FreeBSD port enables OUTCOME_ENABLE_DEPENDENCY_SMOKE_TEST. I guess that's something to add to CI's coverage.

from outcome.

TBH I had assumed if vcpkg was passing it was not a problem. What I was missing is vcpkg doesn't include the addressof changes yet, so we would have encountered that issue then.

from outcome.

Outstanding. Thanks for your patience, sorry it's been such a long trail. It had been my fault all along.

Can I close this ticket?

from outcome.

Sure, no problem at all!

Thanks for fixing this.

from outcome.

Thanks @yurivict . BTW I haven't forgotten LLFIO on FreeBSD, it's just I haven't had any free mornings before work recently, so I haven't been able to make forward progress. It does compile, it's just an unacceptable number of tests fail.

from outcome.

Thanks @ned14
No problem, please do this what it is comfortable. No rush.

Best,
Yuri

from outcome.

@yurivict After the havoc here I thought it might be a good idea to briefly check if I encounter similar issues during the vcpkg port update. And indeed I came across the fact that PROJECT_IS_DEPENDENCY had been replaced with ${PROJECT_NAME}_IS_DEPENDENCY (i.e. outcome_IS_DEPENDENCY in this case). I suggest configuring with -Doutcome_IS_DEPENDENCY=ON as that will completely disable precompiled headers. I'm unsure whether the INTERFACE precompiled headers would propagate into the installed target configs.

from outcome.

@ned14 With -Doutcome_IS_DEPENDENCY=ON tests aren't run, though (through the "test" target).
Currently they all run fine.

from outcome.