Skip to content

Add Windows CI build too#2

Open
ClausKlein wants to merge 21 commits intodevelopfrom
feature/add-windows-ci
Open

Add Windows CI build too#2
ClausKlein wants to merge 21 commits intodevelopfrom
feature/add-windows-ci

Conversation

@ClausKlein
Copy link
Owner

@ClausKlein ClausKlein commented Jan 10, 2026

This offers a portable implementation version that compiles on all OS with all C++23 compilers

  • Refactory CXX_MODULE according to Daniela advice
  • Quickfix: do not use the old implementation
  • Autoupdate pre-commit hooks
  • Use import std; too
  • Build always an interface library

TODO:

  • Windows DLL no lib found
  • Linker problems with gcc on OSX
  • Liner problems with clang Debug on CI

NOTE: This version compiles on OSX with apple-clang, llvm-clang, and g++-15 too!

Following Issues will be fixed:

I have been prepared export headers to test with DLL's on Windows too.

see https://discourse.cmake.org/t/linker-error-with-cxx-modules/15449

@ClausKlein ClausKlein self-assigned this Jan 10, 2026
and portable C++ feature checks
@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from ab326ee to f3c5c4a Compare January 10, 2026 18:19
@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 3415d2e to 01a4dcf Compare January 10, 2026 19:23
This compiles with clang++-21 on OSX
and should build on Windows too.
@coveralls
Copy link

coveralls commented Jan 11, 2026

Pull Request Test Coverage Report for Build 22622218215

Details

  • 26 of 27 (96.3%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-3.7%) to 96.296%

Changes Missing Coverage Covered Lines Changed/Added Lines %
include/beman/scope/scope_impl.hpp 26 27 96.3%
Totals Coverage Status
Change from base Build 22584283472: -3.7%
Covered Lines: 26
Relevant Lines: 27

💛 - Coveralls

@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from f827b5d to 2f5ab94 Compare January 13, 2026 20:05
@ClausKlein
Copy link
Owner Author

ClausKlein commented Jan 13, 2026

Note: Line coverage is only in range of about 95 - 97 % !

bash-5.3$ make coverage
mkdir -p build/coverage
gcovr # XXX -v
(INFO) Reading coverage data...
(INFO) Writing coverage report...
lines: 95.3% (241 out of 253)
functions: 100.0% (173 out of 173)
branches: 49.8% (131 out of 263)
bash-5.3$ 

@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 350bcd8 to 1a86a99 Compare January 15, 2026 21:05
@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 79cd72c to f51bfa4 Compare January 16, 2026 23:09
Prevent warning: placeholder variables are a C++2c extension
@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 26120f1 to 7af734d Compare January 17, 2026 18:52

#include <experimental/scope> //todo unconditional for unique_resource
// detect standard header first, then experimental, otherwise use local implementation
#if defined(__has_include)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is c++17 feature so we don't need an existence check as the above code will already handle that we're 20+

#include <experimental/scope> //todo unconditional for unique_resource
// detect standard header first, then experimental, otherwise use local implementation
#if defined(__has_include)
# if __has_include(<scope>)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No implementation has this header - so this isn't needed.

# else
// no std scope header — fall through to local implementation below
# endif
#elif defined(__cpp_lib_scope) && __cpp_lib_scope >= 2023xxxxL

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again - this doesn't exist at all

@@ -0,0 +1,330 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really prefer the implementation to stay in scope.hpp

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand right: I should do git mv scope_impl.hpp scope.hpp ?

# error "C++20 or later is required"
#endif

// detect standard header first, then experimental, otherwise use local implementation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think now is the time to use the local first and only use experimental on request - aka a cmake override

// Move assignment
constexpr auto operator=(scope_success&& other) noexcept(std::is_nothrow_move_assignable_v<F>)
-> scope_success& = delete;
#if MOVE_ASSIGNMENT_NEEDED

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets use concepts -- in 2026 I don't want macros for this

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried to test them, the move assignment does not work and is not needed I think.

@JeffGarland
Copy link

My review is incomplete, but I'm out of time for now

Comment on lines +291 to +296
// TODO(CK): missing usecase?
constexpr auto get_deleter() const noexcept -> Deleter;

// Helper to tests is_active()
// NOTE: check if active; not required from LWG?
constexpr explicit operator bool() const noexcept { return active; }
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this!

@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 86c2cfa to c0e2002 Compare February 27, 2026 10:15
Quickfix: do not use the old implementation

Autoupdate pre-commit hooks
@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 6475182 to 3d810f1 Compare February 27, 2026 20:36
CMAKE_EXPERIMENTAL_CXX_IMPORT_STD is still needed

Build and install always a header only lib

Fix some clang-tidy warnings

pre-commit autoupdate
@ClausKlein ClausKlein force-pushed the feature/add-windows-ci branch from 743105f to 4a35cc4 Compare March 3, 2026 10:31
@ClausKlein ClausKlein requested review from JeffGarland and PeterSommerlad and removed request for JeffGarland March 3, 2026 12:28
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.

3 participants