Skip to content

CI: cover MSVC, Clang-CL and GCC (+ the MSVC C++26 fix that unblocks it) - #12

Merged
psiha merged 4 commits into
masterfrom
ci/widen-matrix
Aug 12, 2026
Merged

CI: cover MSVC, Clang-CL and GCC (+ the MSVC C++26 fix that unblocks it)#12
psiha merged 4 commits into
masterfrom
ci/widen-matrix

Conversation

@psiha

@psiha psiha commented Aug 12, 2026

Copy link
Copy Markdown
Owner

The matrix was Clang + Apple-Clang. psi.sweater — which consumes this library — already builds it under MSVC, Clang-CL and GCC too, so the downstream has been testing functionoid more thoroughly than functionoid tests itself. That is how #8's gcc constrained-out-of-class-definition bug was found: in the consumer's PR, not here.

Adds MSVC + Clang-CL on windows-2025 (which carries Visual Studio 2026 since the runner-images migration in June 2026), and GCC via the gcc:16 container — the newest released major. 4 lanes → 10.

The MSVC fix is the interesting part

Windows could not have worked before this, and the reason is worth recording: set( CMAKE_CXX_STANDARD 26 ) does nothing under MSVC. CMake has no cxx_std_26 mapping for it, so the flag is silently dropped and the build proceeds at the default dialect. The first error is then language feature 'nested-namespace-definition' requires compiler flag '/std:c++17' — which reads as though the library were C++14 code, 85 errors deep, nowhere near the cause.

CMAKE_CXX_STANDARD_REQUIRED does not help; it just converts the silent decay into a hard configure error:

Target "functionoid_smoke" requires the language dialect "CXX26" (with compiler
extensions). But the current compiler "MSVC" does not support this, or CMake does
not know the flags to enable it.

So the dialect is requested explicitly, keyed on the compiler id (Clang-CL reports Clang and needs none of it — CMake knows its flags — while the MSVC variable is set for clang-cl too, so it would be the wrong test).

With that one change, MSVC builds the library and passes 23/23.

Verified locally before pushing

lane result
MSVC — Release (VS 2026) ✅ 23/23
Clang-CL — Release ✅ 23/23
GCC 16 — Debug ✅ pass
GCC 16 — Release 🟥 SEGFAULT

About that GCC Release lane — read before merging

It is red on master as it stands, not on anything in this PR. It is the pre-existing anomaly documented in #8: CallableMoveOnly.AssignFromStatelessLambda and NestedCallable.SameTraitsCopyAndAssignmentStillCorrect fail under gcc-16 at -O2 -DNDEBUG only — cured by -fno-ipa-modref or -fno-tree-dse or -fno-inline, not by -fno-strict-aliasing; ASan/UBSan clean and masking it; shaped like a gcc-16 ipa-modref/DSE regression deleting a live store. At -O0/-O1/plain -O2 everything passes. In this Release configuration the whole binary now segfaults rather than failing two assertions.

I have deliberately not pinned it away to gcc:15 to get a green tick, because the lane is reporting something true: the library miscompiles under the current GCC release in the configuration users would ship. Three ways forward, your call:

  1. Merge red and treat the GCC-Release lane as the tracking signal until the anomaly is fixed or a gcc bug is filed.
  2. Pin gcc:15 now, land green, and raise gcc:16 separately once the anomaly is resolved.
  3. Merge with GCC-Release marked non-blocking (continue-on-error), keeping it visible without gating other work.

I would avoid a fourth option — dropping -DNDEBUG or weakening the lane so it passes — since that removes exactly the coverage the lane was added for.

psiha added 2 commits August 12, 2026 12:56
`set( CMAKE_CXX_STANDARD 26 )` alone does nothing under MSVC: CMake has no
cxx_std_26 mapping for that compiler, so the flag is silently DROPPED and the
build proceeds at the default dialect. The failure then surfaces far from the
cause - the first error is that a nested namespace definition needs
/std:c++17, which reads as if the library were C++14 code.

Asking for CMAKE_CXX_STANDARD_REQUIRED does not fix it either; it converts the
silent decay into a hard configure error ("the current compiler MSVC does not
support this, or CMake does not know the flags to enable it"). So the dialect
has to be requested directly.

With it, MSVC builds the library and passes the full suite. Clang-CL reports
its compiler id as Clang and needs none of this - CMake knows its flags - so
the branch is keyed on the id rather than on the MSVC variable, which is also
set for clang-cl.
The matrix was Clang + Apple-Clang, while psi.sweater - which CONSUMES this
library - already builds it under MSVC, Clang-CL and GCC as well. The
downstream was therefore testing functionoid more thoroughly than functionoid
tests itself, and library-level breakage surfaced in the consumer's pull
requests: the gcc constrained-out-of-class-definition bug fixed in #8 was
found that way.

Adds, on top of the existing lanes:
  * MSVC and Clang-CL on windows-2025 (which carries Visual Studio 2026 since
    the runner-images migration in June 2026). Windows had no coverage at all
    even though the library is shipped on it.
  * GCC via the gcc:16 container - the newest released major.

Both Windows lanes use multi-config generators, so they take neither an
explicit compiler nor CMAKE_BUILD_TYPE; the configuration is selected at
build/test time through --config / -C, and configure is split per platform
rather than threading shell-specific syntax through one step.

Locally, MSVC and Clang-CL pass 23/23 in Release, and GCC 16 passes in Debug.
GCC 16 Release does NOT pass - see the pull request discussion - and is
included deliberately rather than pinned away, because the failure is real.
@psiha

psiha commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

Verification note on the MSVC fix, since "it builds now" is weak evidence — checked at the generated-artifact level rather than by pass/fail.

test/functionoid_smoke.vcxproj, same source, same generator (VS 2026), toolset 14.51.36231:

dialect in the project file
without the fix no <LanguageStandard> element at all
with the fix <LanguageStandard>stdcpplatest

So CMake really does emit nothing for cxx_std_26 under MSVC — the decay is silent at generate time, not a compile-time override. Consistent with the CI log carrying no D9025 "overriding /std:..." warning: nothing is being overridden, because there was no other flag.

CI runs the same toolchain that fails locally without the fix (MSVC 19.51.36252.0, toolset 14.51.36231), so the local result transfers directly.

One thing I could not explain: psi.sweater's MSVC lane is green with the identical psi.build code path and no explicit dialect flag. Its lane does compile and run its suite, so the difference is real but not in the shared build options — possibly its test target picks the dialect up differently. Flagging it rather than guessing; it does not affect this PR, but it may mean a more principled fix exists (and one that belongs in psi.build, benefiting psi.vm and psi.sweater too, rather than being repeated per repo).

vtable_attrs_test.cpp defines PSI_FUNCTIONOID_DETAIL_INVOKE_FN_ATTR, which
changes the DECLARATION of the vtable's invoke slot. Linking it beside five
TUs compiled without that macro makes the same class template two different
types - an ODR violation, and one whose symptom would land in the OTHER
tests rather than in this one, since GCC may propagate the surviving `pure`
declaration to callers compiled believing the call writes memory.

Nothing observed pins this to a specific failure today, but it is unsound as
written and exactly the kind of thing that produces a bug report of the form
"unrelated test crashes under -O2". Keeping the TU in its own executable also
keeps the attribute honest: `pure` holds for that test's stateless target,
not for callables in general.
@psiha

psiha commented Aug 12, 2026

Copy link
Copy Markdown
Owner Author

GCC lane fixed — and it is a compiler bug, not ours. Two commits.

The crash

Reduced to a single TU, ~20 lines, no gtest:

#include <psi/functionoid/functionoid.hpp>
namespace pf = psi::functionoid;
struct move_only_traits : pf::default_traits {
    static constexpr auto copyable   = pf::support_level::na;
    static constexpr auto moveable   = pf::support_level::nofail;
    static constexpr auto destructor = pf::support_level::trivial;
};
int main() {
    int hits{ 0 };
    pf::callable<void(), move_only_traits> fn;
    fn = [&]() noexcept { ++hits; };
    fn();                                  // SIGSEGV
    return hits == 1 ? 0 : 2;
}
build result
gcc-16 -O0 / -O1 / -O2
gcc-16 -O2 -DNDEBUG 🟥 SIGSEGV
gcc-16 -O2 -DNDEBUG -fno-ipa-modref
clang-22 -O2 -DNDEBUG

Interprocedural mod/ref analysis drops the store that places the small target into the function buffer, so the invoke reads a garbage functor. Both -O2 and NDEBUG are required.

Ruled out on our side first

  • an ODR clash on the vtable attribute macrosvtable_attrs_test.cpp defines PSI_FUNCTIONOID_DETAIL_INVOKE_FN_ATTR, changing the declaration of the invoke slot, while being linked beside five TUs that don't. That is a genuine ODR violation and is fixed in the first commit — but it is not this crash: the segfault survives the fix.
  • BOOST_ASSUME — neutralised wholesale via a forced include; still crashes. So it is not an assumption the code gets wrong.
  • the __restrict on invoke_impl's target reference (which does alias its buffer parameter) — removed; still crashes.
  • -fno-strict-aliasing and -fno-lifetime-dse — neither helps.

Only -fno-ipa-modref does, so that one pass is disabled for GNU. It is the smallest change that keeps the library correct here, not a blanket de-optimization — and it is scoped so clang, clang-cl and MSVC are untouched.

Verified locally, 5/5 clean runs on gcc-16 Release, and clang/gcc Debug+Release all still green. Worth filing upstream with the reproducer above; I have not done that.

gcc-16 miscompiles the callable at -O2 -DNDEBUG: interprocedural mod/ref
analysis drops the placement-new that stores a small target into the function
buffer, so the following invoke reads a garbage target and segfaults.

Reduced to a single translation unit, ~20 lines - a move-only callable
assigned a reference-capturing lambda, then invoked. Both -O2 and NDEBUG are
required; -O0, -O1 and -O2 without NDEBUG are fine, and clang at the same
settings is unaffected.

Ruled out on our side before blaming the compiler:
  * an ODR clash on the vtable attribute macros - real, fixed in the
    preceding commit, and NOT the cause of this;
  * BOOST_ASSUME - every one of them neutralised wholesale, still crashes;
  * the __restrict on invoke_impl's target reference;
  * -fno-strict-aliasing and -fno-lifetime-dse, neither of which helps.

Only ipa-modref is implicated, so rather than disabling the pass for the whole
project the workaround is a per-function attribute on the primitives that
write the function buffer - each manager's assign/clone/move.

Annotating either END of the lost store cures it, so the choice is which to
pessimize: those primitives are one placement-new each on the assignment
path, whereas the invoker thunk is on the hot path taken by every call. The
invoke path is therefore left fully optimized.

The boundary is "every function that writes the function buffer" rather than
the smallest set that happens to make today's tests pass: a bisected subset
would leave the next target shape to crash silently, since the manager is
selected by the target's size and triviality.

Scoped to GCC 16 exactly - the only release observed to need it - so the
workaround expires by itself. A later GCC compiles the annotation away, and
should the miscompile still be there the GCC lane says so loudly instead of
the library quietly carrying a pessimization forever.
@psiha
psiha merged commit c57ebd0 into master Aug 12, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant