-
-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tests fail to build #288
Comments
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? |
It's puzzling, I can't see what is wrong. |
Your clang is trying to
So, somehow this isn't working on your clang. I would suspect the |
(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) |
What clang version is used in the tests? |
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. |
I don't use templates intensely so I didn't notice this. |
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. |
@ned14 |
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 FWIW:
|
Do you know what would be a testcase for the |
That's a good point - this is only occurring on @yurivict's build system when 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 |
I mean, does:
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. |
This code:
prints same addresses for p1 and p2.
I do this for every port I work on. My compiler is clang-15.0.7 |
You could try including all the headers which |
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:
and upload core-result.cpp.i somewhere.
So was I, and it worked just fine. So the compiler version isn't sufficient, nor is it necessarily necessary. |
Here is core-result.cpp.i from my FreeBSD 13.2 system. |
Why on earth is your clang implementing
Your preprocessed output appears to not be defining Have you tried disabling preprocessed headers? Outcome's |
Same reason you switched from using
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 |
This uses outcome-2.2.7 sources verbatim. No patches are applied. |
Anybody who defines an overload for taking the address of a 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.
It is very important that preprocessed headers are not used for
Obviously cmake is doing what it's told on systems which are not @yurivict's. Here are the lines which disable precompiled headers for
EXCEPT here's the thing: it's been omitted from the first set by accident. @yurivict Try adding |
The dependency smoke test is defined way to late for that, aint it? Line 363 in 0c5b665
|
I've stopped precompiled headers being applied to that test in develop branch. |
@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:
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
)
|
Nothing changed. |
Fixed as per your suggestion. |
@yurivict would you mind trying my patch? |
Ah, yes, the FreeBSD port enables OUTCOME_ENABLE_DEPENDENCY_SMOKE_TEST. I guess that's something to add to CI's coverage. |
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. |
Tests are built successfully with rev. 1407d02 |
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? |
Sure, no problem at all! Thanks for fixing this. |
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. |
Thanks @ned14 Best, |
@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 |
@ned14 With |
Version: 2.2.7
clang-15
FreeBSD 13.2
The text was updated successfully, but these errors were encountered: