Conversation
Use $<$<CXX_COMPILER_ID:Clang,AppleClang>:...> so the flag is decided at the consumer's build time rather than at our configure/install time. Without this, installing the package after a Clang configure would leak -freflection-latest into a downstream GCC consumer's build via the exported INTERFACE.
Pin Dockerfile ARG P2996_REF to a specific upstream commit (currently
9ffb96e3ce362289008e14ad2a79a249f58aa90a — today's p2996 branch tip)
so the toolchain stops drifting between rebuilds.
build-image.yml now resolves the ref (from Dockerfile or workflow
input) and tags the built image as both :latest (convenience) and
:p2996-<short SHA> (immutable). Drops the previous :${{ github.sha }}
tag, which was the reflect_cpp26 commit SHA — meaningless for tracking
the toolchain version.
CI still pulls :latest until a follow-up commit switches it to the
pinned tag (which only exists after the next image rebuild).
CI now pulls an immutable, toolchain-versioned tag instead of the floating :latest tag, so test results can no longer change without a corresponding repo change. Image content is identical to the previous :latest (same upstream P2996 SHA, retagged via imagetools).
Non-owning keys (std::string_view, const char*) silently dangled during deserialization — parse_string()'s temporary went out of scope while the view-typed key kept pointing into it.
- docker-build.sh: pass preset as positional arg to inner bash -c instead of interpolating into the command string. - args.hpp: add missing <utility> for std::declval (was relying on transitive include). - tuple.hpp: static_assert tuple size matches struct field count in from_tuple — previously a shorter tuple silently default-initialized the trailing fields. - test_critical_issues.cpp: drop reference to docs/notes/ note that isn't tracked.
- verify-image-pin: pre-flight script + CI job that compares the Dockerfile P2996_REF pin against the image tag in ci.yml. Fails fast on host before paying for a stale-image pull. - install-smoke-test: CI job builds + installs reflect to a temp prefix, then builds a standalone consumer via find_package(reflect) and runs it. Catches export-contract bugs (e.g. compiler-specific INTERFACE flag leakage) that source-tree tests miss. - compile_fail_map_string_view: ctest entry that builds an intentionally ill-formed program (from_json of map<string_view,V>) with WILL_FAIL TRUE — guards the json_map concept rejection.
- enum.hpp: guard flags_from_string against empty separator (would loop forever if enum_from_string ever returned a value for ""); regression test added. - verify-image-pin.sh: check every clang-p2996 tag in ci.yml, not just the first — multiple container jobs each carry the pin. - verify-tests-listed.sh: new pre-flight script that fails CI if any tests/test_*.cpp file is missing from REFLECT_TEST_SOURCES (or vice versa). The CMakeLists comment claimed missing entries surfaced as build failures; they actually silent-skip without this guard. - ci.yml: renamed the verification job to "preflight" and added the new test-list check as a second step. - test_critical_issues.cpp: drop the stale source comment that referenced the untracked docs/notes/ note.
Code-review follow-up release. No new features — hardening, reproducibility, and a couple of latent-bug fixes. CMake / install - Wrap -freflection-latest in a generator expression so the flag is only injected for Clang consumers, not exported to GCC consumers via the installed INTERFACE. - args.hpp: add missing <utility> for std::declval (was relying on a transitive include). Library correctness - json: tighten json_map concept to require std::string keys; the previous concept admitted non-owning keys (std::string_view, const char*) that would silently dangle after deserialization. - tuple: from_tuple now static_asserts that tuple size matches the struct's field count; a shorter tuple previously default-init'd the trailing fields silently. - enum: flags_from_string rejects an empty separator instead of potentially looping forever. Toolchain reproducibility - Pin Dockerfile ARG P2996_REF to a specific upstream SHA; tag the image as :p2996-<short SHA> in build-image.yml. - ci.yml pulls the immutable :p2996-<short SHA> tag instead of :latest; pre-flight script verifies the two pins stay in sync. - Bump actions/checkout v4 -> v5 (Node 24). CI verification scaffolding - Pre-flight checks job (image-pin sync + test-list completeness). - Install + consumer smoke test job (find_package(reflect) + target_link_libraries(reflect::reflect) end-to-end). - Compile-failure ctest entry guarding the json_map concept rejection. Cleanup - docker-build.sh: pass preset as a positional arg instead of interpolating into bash -c. - test_critical_issues.cpp: drop dead reference to docs/notes/ note.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Code-review follow-up release. No new features — hardening, reproducibility, and a couple of latent-bug fixes.
CMake / install
-freflection-latestin a generator expression so it's only injected for Clang consumers — no leakage into a downstream GCC consumer's build via the exported INTERFACE.args.hpp: add missing<utility>forstd::declval(was relying on a transitive include).Library correctness
json: tightenjson_mapconcept to requirestd::stringkeys. Previously acceptedstd::string_viewandconst char*, which would silently dangle after deserialization (parse_string returns a temporary, the view points into it, the temporary dies at end of iteration).tuple:from_tuplenowstatic_asserts that tuple size matches the struct's field count. A shorter tuple previously default-initialized the trailing fields silently.enum:flags_from_stringrejects an empty separator instead of potentially looping forever.Toolchain reproducibility
ARG P2996_REFto a specific upstream SHA; tag the image as:p2996-<short SHA>inbuild-image.yml.ci.ymlpulls the immutable:p2996-<short SHA>tag instead of:latest; pre-flight script verifies the two pins stay in sync.actions/checkoutv4 -> v5 (Node 24).CI verification scaffolding
find_package(reflect)+target_link_libraries(reflect::reflect)end-to-end).json_mapconcept rejection.Cleanup
docker-build.sh: pass preset as a positional arg instead of interpolating intobash -c.test_critical_issues.cpp: drop dead reference todocs/notes/note.Test plan
from_json<map<string_view, int>>passes (i.e., it fails to compile as expected)v0.2.1on main