Merge pull request #1 from shoom1/develop

Release v0.2.1

Author: shoom1

.github/workflows/build-image.yml

@@ -26,7 +26,7 @@ jobs:
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
 
       # GitHub-hosted runners ship with ~14 GB free; LLVM source +
       # build + image export easily exceeds that. Free ~25 GB by
@@ -53,6 +53,22 @@ jobs:
           username: ${{ github.actor }}
           password: ${{ secrets.GITHUB_TOKEN }}
 
+      # Resolve the P2996 ref we'll actually build. Default: read the
+      # pinned ARG P2996_REF from Dockerfile. Override: workflow input.
+      # The resolved SHA drives the image tag so CI can pin to a
+      # toolchain-versioned tag instead of :latest.
+      - name: Resolve P2996 ref
+        id: p2996
+        run: |
+          if [ -n "${{ inputs.p2996_ref }}" ]; then
+            REF="${{ inputs.p2996_ref }}"
+          else
+            REF=$(grep -E '^ARG P2996_REF=' Dockerfile | head -1 | cut -d= -f2)
+          fi
+          echo "ref=$REF" >> "$GITHUB_OUTPUT"
+          echo "short=${REF:0:12}" >> "$GITHUB_OUTPUT"
+          echo "Building with P2996_REF=$REF (tag suffix: ${REF:0:12})"
+
       - name: Build and push
         uses: docker/build-push-action@v5
         with:
@@ -62,7 +78,9 @@ jobs:
           # CI runners have ~7 GB RAM and 2 CPUs — default COMPILE_JOBS=2,
           # LINK_JOBS=1 fits comfortably. Override here if runners change.
           build-args: |
-            ${{ inputs.p2996_ref != '' && format('P2996_REF={0}', inputs.p2996_ref) || '' }}
+            P2996_REF=${{ steps.p2996.outputs.ref }}
+          # :latest floats and is convenient for ad-hoc pulls.
+          # :p2996-<short SHA> is the immutable tag CI pins to.
           tags: |
             ghcr.io/${{ github.repository_owner }}/clang-p2996:latest
-            ghcr.io/${{ github.repository_owner }}/clang-p2996:${{ github.sha }}
+            ghcr.io/${{ github.repository_owner }}/clang-p2996:p2996-${{ steps.p2996.outputs.short }}

.github/workflows/ci.yml

@@ -2,7 +2,7 @@ name: CI
 
 on:
   push:
-    branches: [main]
+    branches: [main, develop]
   pull_request:
   workflow_dispatch:
 
@@ -15,22 +15,40 @@ permissions:
   packages: read
 
 jobs:
+  # Cheap host-level pre-flight checks. Cumulatively run in seconds and
+  # gate the heavier container jobs so a misconfiguration fails fast on
+  # the host instead of after a slow image pull.
+  preflight:
+    name: Pre-flight checks
+    runs-on: ubuntu-latest
+    timeout-minutes: 2
+    steps:
+      - uses: actions/checkout@v5
+      - name: Verify CI image tag matches Dockerfile pin
+        run: ./scripts/verify-image-pin.sh
+      - name: Verify all test files are listed in CMakeLists.txt
+        run: ./scripts/verify-tests-listed.sh
+
   build-and-test:
     name: Build & test (Bloomberg Clang P2996 fork)
     runs-on: ubuntu-latest
     timeout-minutes: 30
+    needs: preflight
 
-    # Pull the prebuilt P2996 image from GHCR. To rebuild it, run the
-    # "Build clang-p2996 image" workflow manually (Actions tab).
+    # Pull the prebuilt P2996 image from GHCR. The :p2996-<short SHA>
+    # tag is immutable and must match the ARG P2996_REF pin in
+    # Dockerfile — bumping the P2996 ref is a coordinated change across
+    # both files plus a workflow-dispatched rebuild of the "Build
+    # clang-p2996 image" workflow. See Dockerfile for the full procedure.
     container:
-      image: ghcr.io/${{ github.repository_owner }}/clang-p2996:latest
+      image: ghcr.io/${{ github.repository_owner }}/clang-p2996:p2996-9ffb96e3ce36
       credentials:
         username: ${{ github.actor }}
         password: ${{ secrets.GITHUB_TOKEN }}
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v4
+        uses: actions/checkout@v5
 
       - name: Configure
         run: |
@@ -45,3 +63,50 @@ jobs:
 
       - name: Run tests
         run: ctest --test-dir build --output-on-failure
+
+  # Verify the installed package is consumable from a fresh build
+  # directory via find_package(reflect). Catches export-contract bugs
+  # (e.g. compiler-specific flags leaking through the INTERFACE) that
+  # in-source-tree tests miss because they don't go through install.
+  install-smoke-test:
+    name: Install + consumer smoke test
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    needs: preflight
+
+    container:
+      image: ghcr.io/${{ github.repository_owner }}/clang-p2996:p2996-9ffb96e3ce36
+      credentials:
+        username: ${{ github.actor }}
+        password: ${{ secrets.GITHUB_TOKEN }}
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v5
+
+      - name: Build & install reflect to temp prefix
+        run: |
+          cmake -S . -B build -G Ninja \
+            -DCMAKE_BUILD_TYPE=Release \
+            -DCMAKE_CXX_COMPILER=clang++ \
+            -DCMAKE_CXX_FLAGS="-stdlib=libc++" \
+            -DCMAKE_EXE_LINKER_FLAGS="-stdlib=libc++" \
+            -DCMAKE_INSTALL_PREFIX=/tmp/reflect-install \
+            -DREFLECT_BUILD_TESTS=OFF \
+            -DREFLECT_BUILD_EXAMPLES=OFF \
+            -DREFLECT_BUILD_BENCHMARKS=OFF
+          cmake --build build --parallel
+          cmake --install build
+
+      - name: Configure & build consumer against installed package
+        run: |
+          cmake -S tests/install-smoke-test -B build-consumer -G Ninja \
+            -DCMAKE_BUILD_TYPE=Release \
+            -DCMAKE_CXX_COMPILER=clang++ \
+            -DCMAKE_CXX_FLAGS="-stdlib=libc++" \
+            -DCMAKE_EXE_LINKER_FLAGS="-stdlib=libc++" \
+            -DCMAKE_PREFIX_PATH=/tmp/reflect-install
+          cmake --build build-consumer --parallel
+
+      - name: Run consumer
+        run: ./build-consumer/consumer

CMakeLists.txt

@@ -1,6 +1,6 @@
 cmake_minimum_required(VERSION 3.20)
 project(reflect
-    VERSION 0.2.0
+    VERSION 0.2.1
     DESCRIPTION "C++26 reflection utilities — header-only, zero dependencies"
     LANGUAGES CXX
 )
@@ -17,16 +17,21 @@ target_include_directories(reflect INTERFACE
 # configure error ("cxx_std_26 is not known to CXX compiler").
 target_compile_options(reflect INTERFACE -std=c++26)
 
-# Detect compiler and add reflection flags
-if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-    # Bloomberg Clang fork
-    target_compile_options(reflect INTERFACE -freflection-latest)
-elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
-    # GCC 16+ — reflection is enabled automatically with -std=c++26
-    if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS "16.0")
-        message(WARNING "GCC < 16 does not support C++26 reflection. "
-                        "Build from GCC trunk or use Bloomberg Clang fork.")
-    endif()
+# Compiler-specific reflection flag. Wrapped in a generator expression
+# so the decision is made at the *consumer's* build time, not when this
+# package is configured/installed. Without this, installing after
+# configuring with Clang would leak -freflection-latest into a
+# downstream GCC consumer's build via the exported INTERFACE.
+target_compile_options(reflect INTERFACE
+    $<$<CXX_COMPILER_ID:Clang,AppleClang>:-freflection-latest>
+)
+
+# GCC 16+ enables reflection automatically with -std=c++26. This warning
+# fires only at our configure time and is not exported to consumers.
+if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU"
+    AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS "16.0")
+    message(WARNING "GCC < 16 does not support C++26 reflection. "
+                    "Build from GCC trunk or use Bloomberg Clang fork.")
 endif()
 
 if (CMAKE_SOURCE_DIR STREQUAL PROJECT_SOURCE_DIR)
@@ -39,9 +44,9 @@ endif()
 # Listed explicitly rather than file(GLOB ...): a glob runs at configure
 # time, so a new test_xxx.cpp added on disk wouldn't be picked up until
 # CMake re-configures. With an explicit list, adding a test is a 20-second
-# tax for guaranteed correctness — a missing entry here surfaces as a
-# linker/test-discovery failure on the next build, not a silently-skipped
-# test.
+# tax for guaranteed determinism. The downside — a forgotten entry would
+# silently skip the test rather than failing the build — is caught by
+# scripts/verify-tests-listed.sh, run as a CI pre-flight step.
 set(REFLECT_TEST_SOURCES
     tests/test_args.cpp
     tests/test_critical_issues.cpp
@@ -60,6 +65,27 @@ if (REFLECT_BUILD_TESTS AND EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/tests")
         target_link_libraries(${test_target} PRIVATE reflect)
         add_test(NAME ${test_name} COMMAND ${test_target})
     endforeach()
+
+    # Compile-failure tests: each source listed here MUST fail to
+    # compile. The target is EXCLUDE_FROM_ALL (so the main build stays
+    # green) and ctest invokes cmake --build on it expecting non-zero
+    # exit (WILL_FAIL TRUE). A successful compile means the guard the
+    # test exercises has regressed.
+    set(REFLECT_COMPILE_FAIL_SOURCES
+        tests/compile_fail_map_string_view.cpp
+    )
+    foreach(cf_src ${REFLECT_COMPILE_FAIL_SOURCES})
+        get_filename_component(cf_name ${cf_src} NAME_WE)
+        set(cf_target reflect_${cf_name})
+        add_executable(${cf_target} EXCLUDE_FROM_ALL ${cf_src})
+        target_link_libraries(${cf_target} PRIVATE reflect)
+        add_test(
+            NAME ${cf_name}
+            COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}
+                    --target ${cf_target} --config $<CONFIG>
+        )
+        set_tests_properties(${cf_name} PROPERTIES WILL_FAIL TRUE)
+    endforeach()
 endif()
 
 # Examples

Dockerfile

@@ -41,25 +41,30 @@ RUN apt-get update \
         zlib1g-dev \
  && rm -rf /var/lib/apt/lists/*
 
-# Pin to a specific commit for reproducible builds. Update intentionally
-# by changing this value (and recording why in the commit message).
+# Pinned to a specific upstream commit for reproducible builds. Bumping
+# this is a deliberate three-step change:
+#   1. update the SHA below
+#   2. rebuild via the "Build clang-p2996 image" workflow
+#      (it tags the image :p2996-<short SHA>)
+#   3. update the matching tag in .github/workflows/ci.yml
+#
 # To find the latest commit on the p2996 branch:
 #   git ls-remote https://github.com/bloomberg/clang-p2996.git refs/heads/p2996
-ARG P2996_REF=p2996
+ARG P2996_REF=9ffb96e3ce362289008e14ad2a79a249f58aa90a
 ARG INSTALL_PREFIX=/opt/clang-p2996
 ARG COMPILE_JOBS=2
 ARG LINK_JOBS=1
 
 WORKDIR /src
 
-# Shallow clone of the p2996 branch — saves ~80% of clone time/disk vs full
-# history. Then check out the specific commit if requested.
+# Shallow-clone the p2996 branch tip (~80% smaller than full history),
+# then fetch and check out the pinned commit. Direct shallow clone of an
+# arbitrary SHA isn't supported by GitHub's smart-HTTP protocol, so the
+# branch hop is necessary.
 RUN git clone --depth 1 --branch p2996 \
         https://github.com/bloomberg/clang-p2996.git . \
- && if [ "${P2996_REF}" != "p2996" ]; then \
-        git fetch --depth 1 origin "${P2996_REF}" \
-     && git checkout "${P2996_REF}"; \
-    fi
+ && git fetch --depth 1 origin "${P2996_REF}" \
+ && git checkout "${P2996_REF}"
 
 # Configure: clang frontend only, libc++/libc++abi/libunwind runtimes,
 # x86_64 target only, Release mode. This trims the build by an order of

include/reflect/args.hpp

@@ -33,6 +33,7 @@
 #include <stdexcept>
 #include <string>
 #include <string_view>
+#include <utility>
 #include <vector>
 
 namespace reflect {

include/reflect/enum.hpp

@@ -243,6 +243,12 @@ template <enumeration E>
 constexpr std::optional<E> flags_from_string(
     std::string_view str, std::string_view separator = "|")
 {
+    // An empty separator would make str.find(separator) return 0 on
+    // every iteration, which combined with remove_prefix(0) would
+    // never advance — infinite loop. Reject it explicitly.
+    if (separator.empty())
+        return std::nullopt;
+
     using U = std::underlying_type_t<E>;
     U combined = 0;
 

include/reflect/json.hpp

@@ -79,12 +79,18 @@ namespace json_detail {
     }
 
     // ---- type categories for JSON dispatch ----
+    // json_map requires an *owning* std::string key. Accepting non-owning
+    // string-like keys (std::string_view, const char*) would silently
+    // dangle on deserialization — parse_string() returns a std::string
+    // by value, and emplacing it into a view-typed key stores a view
+    // into the temporary. Restricting the concept here keeps serialize
+    // and deserialize symmetric: both paths require owning keys.
     template <typename T>
     concept json_map = requires(T t) {
         typename T::key_type;
         typename T::mapped_type;
         { t.begin() } -> std::input_iterator;
-    } && reflect::is_string_like<typename T::key_type>;
+    } && std::same_as<std::remove_cvref_t<typename T::key_type>, std::string>;
 
     template <typename T>
     concept json_array = reflect::is_range<T> && !json_map<T>;

include/reflect/tuple.hpp

@@ -80,6 +80,14 @@ constexpr auto to_ref_tuple(T& obj) {
 
 template <reflectable T, typename Tuple>
 constexpr T from_tuple(Tuple const& t) {
+    // Require exact size match. A shorter tuple would aggregate-initialize
+    // the leading fields and silently default the rest — surprising for
+    // a "↔" conversion. A longer tuple is already a hard error from the
+    // brace-init list, but we catch it here too with a clearer message.
+    static_assert(
+        std::tuple_size_v<std::remove_cvref_t<Tuple>>
+            == detail::tuple_members_of(^^T).size(),
+        "from_tuple: tuple size must equal the struct's field count.");
     return [&]<std::size_t... Is>(std::index_sequence<Is...>) {
         return T{std::get<Is>(t)...};
     }(std::make_index_sequence<std::tuple_size_v<std::remove_cvref_t<Tuple>>>{});

scripts/docker-build.sh

@@ -28,4 +28,4 @@ docker run --rm \
     -v "$REPO_ROOT":/work \
     -w /work \
     "$IMAGE" \
-    bash -c "cmake --preset $PRESET && cmake --build --preset $PRESET"
+    bash -c 'cmake --preset "$1" && cmake --build --preset "$1"' bash "$PRESET"

scripts/verify-image-pin.sh

@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+# Verify that the clang-p2996 image tag pinned in .github/workflows/ci.yml
+# matches the upstream P2996 ref pinned in Dockerfile. Catches the human
+# error of bumping one without the other (drift between the two files).
+set -euo pipefail
+
+DOCKERFILE_REF=$(grep -E '^ARG P2996_REF=' Dockerfile | head -1 | cut -d= -f2)
+EXPECTED_TAG="p2996-${DOCKERFILE_REF:0:12}"
+
+# Check every clang-p2996 tag reference in ci.yml — there are multiple
+# container jobs, and one could drift while another stays correct.
+count=0
+bad=0
+while IFS= read -r tag; do
+    count=$((count + 1))
+    if [ "$tag" != "$EXPECTED_TAG" ]; then
+        echo "ERROR: ci.yml has clang-p2996:$tag" >&2
+        bad=1
+    fi
+done < <(grep -oE 'clang-p2996:[A-Za-z0-9._-]+' .github/workflows/ci.yml | cut -d: -f2)
+
+if [ "$count" -eq 0 ]; then
+    echo "ERROR: no clang-p2996:<tag> references found in ci.yml" >&2
+    exit 1
+fi
+
+if [ "$bad" -ne 0 ]; then
+    echo "       Dockerfile pins P2996_REF=$DOCKERFILE_REF" >&2
+    echo "       expected all ci.yml tags to be: $EXPECTED_TAG" >&2
+    exit 1
+fi
+
+echo "OK: $count ci.yml tag(s) all match Dockerfile P2996_REF '$DOCKERFILE_REF'"