codecov[bot]
commented
9 months ago Codecov Report
Merging #138 (89aff83) into main (3c908fe) will not change coverage. The diff coverage is
n/a.
Closed topazus closed 8 months ago
codecov[bot]
commented
9 months ago Merging #138 (89aff83) into main (3c908fe) will not change coverage. The diff coverage is
n/a.
tocic
commented
9 months ago Thank you for the contribution! Looks like there's a build problem, see meson / Linux Clang 14, for example. Could you please take a look at it? Also, if the shared check is required for correct build, I suggest you to squash the commits into one.
topazus
commented
9 months ago After looking into the build error
clang++-14 -Ilibsnitch.a.p -I. -I.. -I../include -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++20 -O3 -fPIC -MD -MQ libsnitch.a.p/src_snitch.cpp.o -MF libsnitch.a.p/src_snitch.cpp.o.d -o libsnitch.a.p/src_snitch.cpp.o -c ../src/snitch.cpp
In file included from ../src/snitch.cpp:9:
In file included from ../src/snitch_registry.cpp:7:
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:2320:48: error: call to consteval function 'std::chrono::hh_mm_ss::_S_fractional_width' is not a constant expression
static constexpr unsigned fractional_width = {_S_fractional_width()};
^
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:23[20](https://github.com/snitch-org/snitch/actions/runs/6649803452/job/18071251175?pr=138#step:6:21):48: note: undefined function '_S_fractional_width' cannot be used in a constant expression
/usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/chrono:[22](https://github.com/snitch-org/snitch/actions/runs/6649803452/job/18071251175?pr=138#step:6:23)75:2: note: declared here
_S_fractional_width()
^
1 error generated.It seems that clang 14 does not properly handle consteval (I am not sure) according to the link of https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1043011 . Anyway, I tried to update clang 14 to clang 15 in order to deal with it.
The result showed that all tests passed after switching to use clang 15 in the PR of my forked repo. https://github.com/topazus/snitch/pull/3
cschreib
commented
9 months ago Thank you for spotting and fixing the problem with clang-14. This has come to bite us multiple times at work: clang by default will use the latest libstdc++ available on the system, and that is normally tied to gcc. I guess the GitHub action runners were updated with a newer gcc, and that broke the clang-14 build from under our feet.
A more permanent fix for this would be to install libc++-14 (if sticking to clang-14, or libc++-15 if we move on to clang-15; I don't have a preference), and configure clang to use that instead of "whatever is the latest libstdc++". This is what the CMake workflow does. I don't know how to do it with Meson however.
topazus
commented
9 months ago [1/5] Compiling C++ object libsnitch.a.p/src_snitch.cpp.o
FAILED: libsnitch.a.p/src_snitch.cpp.o
clang++-[15](https://github.com/topazus/snitch/actions/runs/6663734595/job/18110093082?pr=3#step:6:16) -Ilibsnitch.a.p -I. -I.. -I../include -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++[20](https://github.com/topazus/snitch/actions/runs/6663734595/job/18110093082?pr=3#step:6:21) -O3 -stdlib=libc++ -fPIC -MD -MQ libsnitch.a.p/src_snitch.cpp.o -MF libsnitch.a.p/src_snitch.cpp.o.d -o libsnitch.a.p/src_snitch.cpp.o -c ../src/snitch.cpp
In file included from ../src/snitch.cpp:1:
In file included from ../src/snitch_append.cpp:1:
In file included from ../include/snitch/snitch_append.hpp:4:
In file included from ../include/snitch/snitch_concepts.hpp:4:
./snitch/snitch_config.hpp:4:10: fatal error: 'version' file not found
#include <version> // for C++ feature check macros
^~~~~~~~~
1 error generated.A new error appeared when I tried to use libc++ if available. I can the build it successfully on my machine with clang 17 and libc++.
cschreib
commented
9 months ago Hmm that is odd; the CMake workflows run clang-14 with libc++-14, and that works fine. Perhaps libc++ headers are not installed, or not the right version? The CMake workflow includes this step to make sure:
- name: Setup Clang
if: matrix.platform.compiler == 'clang++' && matrix.platform.os == 'ubuntu-latest'
run: sudo apt install clang libc++-dev libc++abi-devAlso, regarding your proposed addition to the Meson scripts:
if cxx.get_id() == 'clang'
# Use libc++ when available when compiling with clang.
if cxx.find_library('libc++', required: false).found()
cpp_arguments += [
'-stdlib=libc++',
]
endif
endifI don't think this is the right approach; it is up to the user of the library to decide which stdlib they want to use, we shouldn't interfere. I think the correct place to change would be in the Meson workflow. Adding a setup step like the one I quoted above, and then somehow telling Meson from the command line to use libc++. Perhaps like this:
name: meson
on:
workflow_dispatch:
pull_request:
push:
branches:
- main
jobs:
meson-build:
strategy:
fail-fast: false
matrix:
platform:
- { name: Linux GCC 12, os: ubuntu-latest, compiler: g++12, cxx: "g++-12", backend: "ninja", build: "linux-libstdc++", args: "", flags: ""}
- { name: Linux GCC 12 nounity, os: ubuntu-latest, compiler: g++12, cxx: "g++-12", backend: "ninja", build: "linux-libstdc++", args: "-Dunity_build=false", flags: ""}
- { name: Linux GCC 12 shared, os: ubuntu-latest, compiler: g++12, cxx: "g++-12", backend: "ninja", build: "linux-libstdc++", args: "--default-library shared", flags: ""}
- { name: Linux Clang 14, os: ubuntu-latest, compiler: clang-14, cxx: "clang++-14", backend: "ninja", build: "linux-libc++", args: "", flags: "-stdlib=libc++"}
- { name: Windows 64, os: windows-latest, compiler: msvc, cxx: "cl", backend: "vs2022 --vsenv", build: "win64-vs2022", args: "", flags: ""}
- { name: MacOS, os: macos-latest, compiler: clang++, cxx: "clang++", backend: "ninja", build: "osx-libc++", args: "", flags: ""}
build-type:
- release
name: ${{matrix.platform.name}} ${{matrix.build-type}} ${{matrix.config.name}}
runs-on: ${{matrix.platform.os}}
defaults:
run:
shell: bash
steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Setup Clang
if: matrix.platform.compiler == 'clang-14' && matrix.platform.os == 'ubuntu-latest'
run: sudo apt install clang-14 libc++-14-dev libc++abi-14-dev
- uses: actions/setup-python@v4
with:
python-version: '3.x'
- run: pip install meson ninja
- name: meson setup
run: CXX=${{matrix.platform.cxx}} CXXFLAGS="${{matrix.platform.flags}}" meson setup ${{matrix.platform.build}} --backend=${{matrix.platform.backend}} -Dbuildtype=${{matrix.build-type}} -Dprefix=`pwd`/../install ${{matrix.platform.args}}
- name: Build
run: meson compile -C ${{matrix.platform.build}}
- name: Install
run: meson install -C ${{matrix.platform.build}}
- name: Test single header
run: meson test -C ${{matrix.platform.build}}Thanks for your detailed suggestions, I have taken them and modified the corresponding files. I added the -stdlib=libc++ compiler or linker flags to meson with the command line option of cpp_args or cpp_link_args.
Ref: https://mesonbuild.com/Builtin-options.html#compiler-options
cschreib
commented
8 months ago Sorry for the delay; I was away for my sister's wedding :) Thank you very much for contributing this change!
The PR is related to one of the issues in #116