Skip to content
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

Feature/c++23 #76

Merged
merged 12 commits into from
Sep 24, 2024
2 changes: 2 additions & 0 deletions .github/workflows/asan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ on:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
Expand Down
9 changes: 8 additions & 1 deletion .github/workflows/clang-format-check.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
name: clang-format Check
on: [push, pull_request]
on:
push:
branches:
- main
- develop
pull_request:
branches:
- develop
Comment on lines +2 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Adjust Workflow Triggers to 'develop' Branch Only

It has been verified that the main branch does not exist in the repository. Please update the workflow triggers to exclusively include the develop branch:

  • Push events should be limited to the develop branch.
  • Pull request events should be restricted to the develop branch.

This alignment ensures that clang-format checks are executed on the correct and existing branch, optimizing your CI/CD processes.

Analysis chain

Workflow trigger optimization looks good, but verify branch strategy.

The changes to the workflow triggers are well-structured and more targeted:

  1. Push events are now limited to 'main' and 'develop' branches.
  2. Pull request events are restricted to the 'develop' branch.

This approach is generally beneficial as it:

  • Reduces unnecessary workflow runs, potentially saving on CI/CD costs and resources.
  • Focuses the clang-format checks on the most critical branches.

To ensure this aligns with your project's branching strategy, please run the following script:

This script will help verify if the 'main' and 'develop' branches are indeed the primary branches used in your repository, and if there are any other frequently used branches that might need to be included in the workflow triggers.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the branching strategy used in the repository

# Check for the existence of main and develop branches
echo "Checking for main and develop branches:"
git branch -r | grep -E 'origin/(main|develop)$'

# Check for recent activity on these branches
echo -e "\nRecent commits on main and develop branches:"
git for-each-ref --sort=-committerdate --format='%(refname:short) %(committerdate:relative)' refs/remotes/origin/main refs/remotes/origin/develop | head -n 5

# Check if there are other frequently used branches
echo -e "\nOther frequently used branches (if any):"
git for-each-ref --sort=-committerdate --format='%(refname:short) %(committerdate:relative)' refs/remotes/origin/* | grep -vE 'origin/(main|develop)$' | head -n 5

Length of output: 795


jobs:
formatting-check:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ on:
- main
- develop
pull_request:
branches:
- develop
schedule:
- cron: '26 7 * * 0'
workflow_dispatch:
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/cpp-linter.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
name: cpp-linter

on: [push, pull_request, workflow_dispatch]
on:
push:
branches:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

jobs:
cpp-linter:
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/cppcheck.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
name: Cppcheck

on: [push, pull_request, workflow_dispatch]
on:
push:
branches:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/linux-clang.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
name: Linux Clang

on: [push, pull_request, workflow_dispatch]
on:
push:
branches:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/linux-gcc.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
name: Linux GCC

on: [push, pull_request, workflow_dispatch]
on:
push:
branches:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/lsan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ on:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/msan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ on:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/sonarcloud.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ on:
- main
- develop
pull_request:
types: [opened, synchronize, reopened]
workflow_dispatch:

concurrency:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/tsan.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ on:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/valgrind.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ on:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

concurrency:
Expand Down
10 changes: 9 additions & 1 deletion .github/workflows/whitespace.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
name: Whitespace

on: [push, pull_request, workflow_dispatch]
on:
push:
branches:
- main
- develop
pull_request:
branches:
- develop
workflow_dispatch:

jobs:
whitespace:
Expand Down
20 changes: 15 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ cache:
- $TRAVIS_BUILD_DIR/vcpkg
- $HOME/.cache/vcpkg/archives

# Don't double build branch PRs
branches:
except:
- /^pr\..*/

addons:
apt:
update: true
Expand All @@ -21,21 +26,21 @@ addons:
- yasm
- gcc-12
- g++-12
- clang-15
- ninja-build
- cppcheck
- doxygen
- graphviz
- lcov
- valgrind
- ccache
- wget

os:
- linux

compiler:
- g++-12
- clang-15
- clang-19
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Clang version updated, but g++ version requires updating.

The update to clang-19 aligns with the PR objective to support C++23. However, the analysis confirms that g++-12 is still being used. To ensure consistent C++23 support across both compilers, consider updating g++ to a more recent version (e.g., g++-13 or g++-14).

Analysis chain

Clang version updated, but g++ version might need attention.

The update to clang-19 is good and aligns with the PR objective to support C++23. However, I notice that g++-12 is still being used. Consider updating g++ to a more recent version (e.g., g++-13 or g++-14) to ensure consistent C++23 support across both compilers.

To check the g++ version references, run:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
echo "g++ version references:"
rg 'g\+\+-\d+' .travis.yml

Length of output: 613


jobs:
fast_finish: true
Expand Down Expand Up @@ -141,9 +146,14 @@ before_install:
fi
- |
if [[ "$CXX" == "clang++" ]]; then
sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-15 90
sudo update-alternatives --config clang
export CXX="clang++-15" CC="clang-15"
wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 19
sudo update-alternatives --install /usr/bin/clang clang /usr/bin/clang-19 100
sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-19 100
export CXX="clang++-19" CC="clang-19"
clang --version
clang++ --version
fi

install:
Expand Down
8 changes: 5 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ set(ENV{CGAL_DATA_DIR} CMAKE_BINARY_DIR/Data)
# add_compile_options(/DNOMINMAX)
#endif()

if (MSVC)
# Add /utf-8 flag for MSVC
add_compile_options(/utf-8)
endif()

# Project vcpkg dependencies

# https://github.com/CGAL/cgal
Expand Down Expand Up @@ -109,9 +114,6 @@ find_package(spdlog CONFIG REQUIRED)
# https://github.com/intel/tbb
find_package(TBB CONFIG REQUIRED)

# https://github.com/TartanLlama/expected
find_package(tl-expected CONFIG REQUIRED)

# https://github.com/TartanLlama/function_ref
find_package(tl-function-ref CONFIG REQUIRED)

Expand Down
2 changes: 1 addition & 1 deletion CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"binaryDir": "${sourceDir}/build",
"cacheVariables": {
"CMAKE_CXX_EXTENSIONS": "OFF",
"CMAKE_CXX_STANDARD": "20",
"CMAKE_CXX_STANDARD": "23",
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

CMake is not installed in the CI environment

The CI environment lacks CMake, which is essential for building the project with the updated C++23 standard. This absence is causing all preset builds to fail.

  • Action Required: Install CMake in the CI environment to enable successful builds with the ci-std preset and ensure compatibility with C++23.
Analysis chain

Approved: C++ standard updated to C++23

The change from C++20 to C++23 aligns with the PR objective of updating the project to utilize C++23 features. This update in the "ci-std" preset will propagate to all configurations that inherit from it.

To ensure compatibility across different build environments, please verify that all targeted compilers and platforms fully support C++23. You may want to run the following command in your CI pipeline to check compiler support:

If any issues arise, consider adding conditional logic to fall back to C++20 for environments that don't yet fully support C++23.


[/run_scripts]

Scripts executed

The following scripts were executed for the analysis:

Script:

cmake --preset=ci-std && cmake --build build

Length of output: 3515


Script:

#!/bin/bash
# Check if cmake is installed
if command -v cmake >/dev/null 2>&1; then
    cmake --version
else
    echo "CMake is not installed."
    exit 1
fi

Length of output: 84

"CMAKE_CXX_STANDARD_REQUIRED": "ON"
}
},
Expand Down
2 changes: 1 addition & 1 deletion cmake/StandardProjectSettings.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ endif()
set(BOOST_MIN_VERSION "1.75.0")

# Use C++20
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD 23)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Potential Compatibility Issues with std::expected

The upgrade to C++23 introduces potential compatibility concerns:

  • Usage of std::expected found in several headers (include/Move_command.hpp, include/Ergodic_moves_3.hpp, include/Apply_move.hpp) is not part of the C++23 standard.
  • Ensure that std::expected is provided by an external library (e.g., Boost) or a custom implementation compatible with C++23.
  • Verify that all dependencies and compiler support the required features for std::expected.
Analysis chain

Upgrade to C++23 standard

The C++ standard has been updated from C++20 to C++23, which aligns with the PR objectives. This change brings access to the latest C++ features and improvements.

However, there are a few considerations:

  1. Ensure that all compilers and tools used in the project support C++23.
  2. Some libraries or dependencies might not be compatible with C++23 yet.
  3. The codebase may need to be reviewed for potential optimizations or refactoring opportunities introduced by C++23.

To verify compiler support and potential issues, run the following script:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check C++23 support in the project

# Test: Check if CMake is configured to use C++23 in other files
echo "Checking for C++23 configuration in other CMake files:"
rg --type cmake "set\(CMAKE_CXX_STANDARD" -g "!StandardProjectSettings.cmake"

# Test: Look for compiler version checks that might need updating
echo "Checking for compiler version checks:"
rg --type cmake "CMAKE_CXX_COMPILER_VERSION|gcc.*version|clang.*version"

# Test: Check for any explicit C++20 flags that might need updating
echo "Checking for explicit C++20 flags:"
rg --type cmake "std=c\+\+20|-std=c\+\+20"

# Test: Look for potential compatibility issues in the code
echo "Checking for potential C++23 compatibility issues:"
rg --type cpp "std::expected|<expected>|std::generator|<generator>|std::mdspan|<mdspan>"

Length of output: 477


Script:

#!/bin/bash
# Description: Check C++23 support in the project

# Test: Check if CMake is configured to use C++23 in other files
echo "Checking for C++23 configuration in other CMake files:"
rg --type cmake 'set\(CMAKE_CXX_STANDARD\s+23\)' --ignore-file StandardProjectSettings.cmake

# Test: Look for compiler version checks that might need updating
echo "Checking for compiler version checks:"
rg --type cmake 'CMAKE_CXX_COMPILER_VERSION|gcc\s+version|clang\s+version'

# Test: Check for any explicit C++20 flags that might need updating
echo "Checking for explicit C++20 flags:"
rg --type cmake 'std=c\+\+20|-std=c\+\+20'

# Test: Look for potential compatibility issues in the code
echo "Checking for potential C++23 compatibility issues:"
rg --type cpp 'std::expected|<expected>|std::generator|<generator>|std::mdspan|<mdspan>'

Length of output: 1310


Script:

#!/bin/bash
# Description: Investigate sources and usages of specific C++ constructs

# Test: Identify files where 'std::expected' is defined or implemented
echo "Searching for definitions of 'std::expected':"
rg --type cpp 'std::expected' --search-zip

# Test: Identify if 'std::generator' and 'std::mdspan' are used from external libraries
echo "Searching for 'std::generator' and 'std::mdspan' usages from external sources:"
rg --type cpp 'std::generator|std::mdspan' | rg -v 'std::'

# Test: Check for external library dependencies that might provide these features
echo "Searching for Boost or other libraries that provide 'expected', 'generator', or 'mdspan':"
rg --files-with-matches 'Boost' --type cmake
rg --files-with-matches 'find_package\(Boost' --type cmake

Length of output: 1121

set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

Expand Down
24 changes: 12 additions & 12 deletions include/Apply_move.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,29 @@
#ifndef CDT_PLUSPLUS_APPLY_MOVE_HPP
#define CDT_PLUSPLUS_APPLY_MOVE_HPP

#include <spdlog/spdlog.h>

Check failure on line 14 in include/Apply_move.hpp

View workflow job for this annotation

GitHub Actions / cpp-linter

include/Apply_move.hpp:14:10 [clang-diagnostic-error]

'spdlog/spdlog.h' file not found

#include <expected>
#include <string>
#include <tl/expected.hpp>
#include <tl/function_ref.hpp>

/// @brief An applicative function similar to std::apply, but on manifolds
/// @tparam ManifoldType The type (topology, dimensionality) of manifold
/// @tparam ExpectedType The result of the move on the manifold
/// @tparam FunctionType The type of move applied to the manifold
/// @param t_manifold The manifold on which to make the Pachner move
/// @param t_move The Pachner move
/// @returns The expected or unexpected result in a tl::expected<T,E>
/// @see https://tl.tartanllama.xyz/en/latest/api/function_ref.html
/// @see https://tl.tartanllama.xyz/en/latest/api/expected.html
/**
* \brief An applicative function similar to std::apply on a manifold
* \tparam ManifoldType The type (topology, dimensionality) of manifold
* \tparam ExpectedType The result type of the move on the manifold
* \tparam FunctionType The type of move applied to the manifold
* \param t_manifold The manifold on which to make the Pachner move
* \param t_move The Pachner move
* \return The expected or unexpected result in a std::expected<T,E>
*/
template <typename ManifoldType,
typename ExpectedType = tl::expected<ManifoldType, std::string>,
typename ExpectedType = std::expected<ManifoldType, std::string>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Missing C++23 Compiler in Build Environment

The build configuration correctly sets CMAKE_CXX_STANDARD to 23, enabling C++23 features like std::expected. However, no C++ compiler (g++ or clang++) was found in the build environment. This absence prevents the project from being compiled with the required C++23 standard, causing the static analysis failure.

Actions to Resolve:

  • Install a C++23-Compatible Compiler: Ensure that a C++23-compatible compiler (e.g., GCC 12+, Clang 16+, or MSVC 19.34+) is installed in the build environment.
  • Configure the Compiler in CMake: Specify the C++ compiler in the CMake configuration if not using the default compiler. For example, set CMAKE_CXX_COMPILER to the path of the installed compiler.
Analysis chain

Update to std::expected might require compiler adjustment

The change from tl::expected to std::expected is correct for the transition to C++23. However, the static analysis failure suggests that your current compiler might not support C++23 features yet.

To resolve this:

  1. Ensure your compiler supports C++23 (e.g., GCC 12+, Clang 16+, or MSVC 19.34+).
  2. Update your build configuration to use C++23.

Let's verify the current compiler and C++ standard setting:

If the compiler version is insufficient or the C++ standard is not set to 23, please update accordingly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check compiler version and C++ standard setting
if command -v g++ &> /dev/null; then
    echo "GCC version:"
    g++ --version | head -n 1
elif command -v clang++ &> /dev/null; then
    echo "Clang version:"
    clang++ --version | head -n 1
fi

echo "Checking C++ standard setting in CMakeLists.txt:"
grep -n "set(CMAKE_CXX_STANDARD" CMakeLists.txt

Length of output: 194


Script:

#!/bin/bash
# Search for CMAKE_CXX_STANDARD in all CMakeLists.txt files
echo "Searching for CMAKE_CXX_STANDARD settings in all CMakeLists.txt files:"
rg "set\s*\(\s*CMAKE_CXX_STANDARD" --type cmake

# Search for compiler flags that set the C++ standard
echo "Searching for compiler flags that set the C++ standard (e.g., -std=c++23):"
rg "-std=c\+\+23" --type cmake

Length of output: 924


Script:

#!/bin/bash
# Search for CMAKE_CXX_STANDARD in all CMakeLists.txt files
echo "Searching for CMAKE_CXX_STANDARD settings in all CMakeLists.txt files:"
rg "set\s*\(\s*CMAKE_CXX_STANDARD\s+23\s*\)" --type cmake

# Search for compiler flags that set the C++ standard
echo "Searching for compiler flags that set the C++ standard (e.g., -std=c++23):"
rg "-std=c\+\+23" --type cmake

Length of output: 862


Script:

#!/bin/bash
# Search for CMAKE_CXX_STANDARD in all CMakeLists.txt files
echo "Searching for CMAKE_CXX_STANDARD settings in all CMakeLists.txt files:"
rg "set\s*\(\s*CMAKE_CXX_STANDARD\s+23\s*\)" --type cmake

# Correctly search for compiler flags that set the C++ standard
echo "Searching for compiler flags that set the C++ standard (e.g., -std=c++23):"
rg '-std=c\+\+23' --type cmake

# Additionally, search for CMAKE_CXX_FLAGS that might include the C++23 standard
echo "Searching for CMAKE_CXX_FLAGS that include the C++23 standard:"
rg "CMAKE_CXX_FLAGS.*-std=c\+\+23" --type cmake

Length of output: 1047


Script:

#!/bin/bash
# Search for the C++ compiler set in CMake files
echo "Searching for CMAKE_CXX_COMPILER settings in all CMakeLists.txt files:"
rg "set\s*\(\s*CMAKE_CXX_COMPILER\s+" --type cmake

# If a compiler is found, display its version
echo "Checking the version of the specified C++ compiler:"
COMPILER=$(rg "set\s*\(\s*CMAKE_CXX_COMPILER\s+" --type cmake | awk -F'"' '{print $2}' | head -n 1)
if [ -n "$COMPILER" ]; then
    if [ -x "$COMPILER" ]; then
        $COMPILER --version | head -n 1
    else
        echo "Compiler path '$COMPILER' is not executable or does not exist."
    fi
else
    # If no compiler is explicitly set, check the default compiler
    echo "No explicit C++ compiler set in CMake files. Checking default compiler:"
    if command -v g++ &> /dev/null; then
        echo "GCC version:"
        g++ --version | head -n 1
    elif command -v clang++ &> /dev/null; then
        echo "Clang version:"
        clang++ --version | head -n 1
    else
        echo "No default C++ compiler (g++ or clang++) found in the environment."
    fi
fi

# Verify the C++ standard setting in CMake
echo "Verifying CMAKE_CXX_STANDARD is set to 23:"
rg "set\s*\(\s*CMAKE_CXX_STANDARD\s+23\s*\)" --type cmake

Length of output: 989

Tools
GitHub Check: build

[failure] 30-30:
no template named 'expected' in namespace 'std'

typename FunctionType = tl::function_ref<ExpectedType(ManifoldType&)>>
auto constexpr apply_move(ManifoldType&& t_manifold,
FunctionType t_move) noexcept -> decltype(auto)
{
if (auto result = std::invoke(t_move, std::forward<ManifoldType>(t_manifold));

Check failure on line 35 in include/Apply_move.hpp

View workflow job for this annotation

GitHub Actions / build

Syntax Error: AST broken, 'if' doesn't have two operands. [internalAstError]

Check failure on line 35 in include/Apply_move.hpp

View workflow job for this annotation

GitHub Actions / build

Syntax Error: AST broken, 'if' doesn't have two operands. [internalAstError]

Check failure on line 35 in include/Apply_move.hpp

View workflow job for this annotation

GitHub Actions / build

Syntax Error: AST broken, 'if' doesn't have two operands. [internalAstError]

Check failure on line 35 in include/Apply_move.hpp

View workflow job for this annotation

GitHub Actions / build

Syntax Error: AST broken, 'if' doesn't have two operands. [internalAstError]
result)
result.has_value())
{
return result;
}
Expand Down
Loading
Loading