Add modern-cpp plugin for C++20/23/26 best practices

[wizardengineer]

Modern C++ skill guiding Claude toward modern idioms with a security emphasis. Mirrors modern-python in spirit but focuses on language standards rather than toolchain.

Features tiered by practical usability:

• Tier 1 (Use Today): C++20/23 features with solid compiler support
• Tier 2 (Deploy Now): Compiler hardening, sanitizers, hardened libc++
• Tier 3 (Plan For): C++26 reflection
• Tier 4 (Watch): Contracts, std::execution

Includes SKILL.md entry point + 6 reference docs:

• anti-patterns.md (30+ legacy-to-modern swaps)
• cpp20-features.md (concepts, ranges, span, format, coroutines)
• cpp23-features.md (expected, print, deducing this, flat_map)
• cpp26-features.md (reflection, contracts, memory safety)
• compiler-hardening.md (flags, sanitizers, hardened libc++)
• safe-idioms.md (security patterns by vulnerability class)

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[frabert]

Very nice 👍

[dguido]

@claude review once

[claude]

🔴 This PR adds .codex/skills/modern-cpp as a symlink to ../../plugins/modern-cpp/skills/modern-cpp, but AGENTS.md (Codex Compatibility section) explicitly forbids adding any .codex/ directory. The sidecar provides no functional benefit because Codex already loads plugins through the standard .claude-plugin/marketplace.json. Please delete .codex/skills/modern-cpp (and the now-empty .codex/ tree) before merging.

Extended reasoning...

The rule. AGENTS.md, under the "Codex Compatibility" section, states:

This repository uses Claude plugin marketplace metadata as the canonical source for both Claude Code and Codex. Codex supports .claude-plugin/marketplace.json and plugins/<name>/.claude-plugin/plugin.json directly, so do not add duplicate Codex-only sidecar metadata.

Rules:

• Do not add .agents/plugins/marketplace.json, .codex/, or plugins/<name>/.codex-plugin/.

The PR introduces exactly one of the three forbidden items: a .codex/ directory containing skills/modern-cpp (a symlink back into plugins/modern-cpp/skills/modern-cpp).

Why it's a new, isolated violation. I ran ls -la .codex/ and ls -la .codex/skills/ on the PR branch: the only entry under .codex/ anywhere in the repository is the new modern-cpp symlink introduced by this PR. There is no pre-existing pattern of Codex sidecar directories — this is a clean, isolated rule break, not Claude conforming to existing convention.

Why the rule exists. The same AGENTS.md section explains the rationale: the canonical Claude marketplace metadata (.claude-plugin/marketplace.json, which this PR already updates correctly) is loaded by Codex directly. The PR's own README change reflects this: "Codex supports Claude plugin marketplaces directly, so this repository does not need Codex-specific sidecar metadata." So .codex/skills/modern-cpp provides zero functional value — it is exactly the "duplicate Codex-only sidecar metadata" the rule forbids.

Step-by-step proof.

1. Check out the PR branch.
2. cat AGENTS.md | grep -A 4 'Rules:' → confirms the prohibition on .codex/.
3. find . -path ./node_modules -prune -o -name '.codex' -print → returns exactly one path: ./.codex, created by this PR.
4. ls -la .codex/skills/ → shows modern-cpp -> ../../plugins/modern-cpp/skills/modern-cpp, the symlink added by the PR's diff hunk /dev/null → .codex/skills/modern-cpp.
5. README.md (also modified by this PR) states Codex loads via Claude marketplace compatibility — corroborating that the sidecar adds no functional value.

Impact. This violates an explicit, documented contributor rule. Other plugins in this repo (modern-python, c-review, etc.) all live under plugins/<name>/ only and Codex loads them fine. Merging this would set a precedent for sidecar directories and invite future drift; it also risks confusing future Codex loadability checks if they ever start enforcing the prohibition.

Fix. Delete the symlink and the empty parent directory:

rm .codex/skills/modern-cpp
rmdir .codex/skills .codex

No other PR changes need to be touched — the plugin works through .claude-plugin/marketplace.json and plugins/modern-cpp/ exactly like every other plugin in the marketplace.

[claude]

🟡 The inline comment on line 77 is technically inaccurate — since C++11's unrestricted unions ([class.union]), declaring a union with a non-trivial member like is well-formed, not UB. The compiler implicitly deletes the union's default constructor/destructor/copy/move, and the actual UB only arises from accessing the wrong active member or mismanaging member lifetimes. Suggest rewording to something like to match the technically-correct framing the preceding comment already uses.

Extended reasoning...

The safe-idioms.md "Variant Over Union" section currently reads:

// DANGEROUS: union — accessing wrong member is UB
union Value { int i; double d; std::string s; };  // UB: string in union

The leading // DANGEROUS comment is correct — accessing the non-active member of a union is UB. But the trailing // UB: string in union comment overstates this and contradicts the standard.

What the standard actually says. Since C++11 (P0426/N2544, "unrestricted unions"), [class.union]/2 explicitly permits a union to contain members with non-trivial special member functions. The declaration union Value { int i; double d; std::string s; }; is well-formed. What changes is that the union's corresponding special member functions (default constructor, destructor, copy/move) become implicitly deleted because std::string has non-trivial counterparts. That means default-constructing or copying a Value is a compile error, not UB. The programmer must explicitly define those special members and use placement-new and explicit ~string() calls to manage the active member.

Why the inline comment is wrong. It frames the declaration as UB. But the declaration compiles cleanly; only certain operations (reading the wrong active member, letting an implicit destructor try to destroy nothing meaningful, failing to construct/destruct via placement-new) are UB. The two comments on this snippet thus contradict each other: the prior comment correctly localizes UB to accessing the wrong member, while the trailing comment incorrectly attributes UB to the union itself.

Step-by-step proof. Compile union Value { int i; double d; std::string s; }; int main() {} with g++ -std=c++20 -Wall -Wextra -Wpedantic — it builds without diagnostics. The translation unit contains a valid union with a std::string member; no UB is invoked. Now extend it to int main() { Value v; } — this fails to compile, with a diagnostic like "use of deleted function Value::Value()". That demonstrates the standard's actual behavior: implicit deletion of the default constructor, not undefined behavior. Now write int main() { Value v; new (&v.s) std::string("ok"); std::cout << v.i; v.s.~basic_string(); } — this compiles, runs, and is UB only at the read of v.i (a non-active member), which is exactly what the prior // DANGEROUS comment already captures.

Impact. This is teaching content for a skill that explicitly aims to guide users toward "technically-correct framing" of UB rules. Readers internalize the wording. Misstating which construct is UB undermines the skill's pedagogical purpose, and the inaccuracy is small enough to easily fix without rewriting the section.

Suggested fix. Drop the trailing inline comment or replace it with one that matches the preceding correct framing, e.g.:

// DANGEROUS: union — accessing wrong active member is UB
union Value { int i; double d; std::string s; };  // non-trivial member: lifetime must be managed manually (placement new + explicit dtor)

[claude]

🟡 The deducing-this example in cpp23-features.md (line 119-123) uses std::forward<Self>(self).data_[i], which only collapses the const/non-const overloads but silently drops value category for rvalue Self — std::vector<char>::operator[] has no ref-qualified overload, so it always returns char& regardless of how the vector is qualified. The canonical C++23 idiom for forwarding a subobject access (introduced in P2445 specifically for this case) is std::forward_like<Self>(self.data_[i]). Since this doc teaches the modern C++23 idiom and the BEFORE comment explicitly promises to collapse all four "const/non-const x lvalue/rvalue" overloads, switching to std::forward_like would make the example actually deliver on that promise.

Extended reasoning...

What the bug is

The C++23 "deducing this" example in cpp23-features.md lines 113-123 shows:

class Buffer {
    std::vector<char> data_;
public:
    template <typename Self>
    auto&& operator[](this Self&& self, size_t i) {
        return std::forward<Self>(self).data_[i];
    }
};

The inline comment in the BEFORE example explicitly promises this collapses "Often 4 overloads: const/non-const x lvalue/rvalue". But the AFTER code only collapses 2 of those 4 correctly. The rvalue cases silently drop their value category.

Why it's wrong

std::vector<T>::operator[] is not ref-qualified — it has only the single overload T& operator[](size_t) (plus a const version) that returns T& regardless of whether the vector instance is an lvalue or rvalue. So when the cast on the object expression hits an unqualified member function, the rvalue-ness is consumed and lost.

Step-by-step proof

Walk through the four cases of Self:

1. Self = Buffer&:

   • std::forward<Buffer&>(self) → Buffer& (lvalue)
   • .data_ → vector<char>& (lvalue)
   • [i] invokes the non-const operator[] → returns char& ✓

2. Self = const Buffer&:

   • std::forward<const Buffer&>(self) → const Buffer&
   • .data_ → const vector<char>&
   • [i] invokes the const operator[] → returns const char& ✓

3. Self = Buffer (rvalue):

   • std::forward<Buffer>(self) → Buffer&& (xvalue)
   • .data_ → vector<char>&& (xvalue, member access preserves category)
   • [i] invokes operator[] — no rvalue overload exists — returns char& (lvalue) ✗
   • The xvalue-ness of self is dropped at the subscript

4. Self = const Buffer (rvalue):

   • Same as above, but returns const char& instead of const char&& ✗

In other words, value category is preserved through the member access .data_, but then immediately consumed by an operator that doesn't know how to honor it.

What the fix looks like

C++23 added std::forward_like (P2445R1) precisely to solve this 'forward category of one object to a subobject access' problem. The body becomes:

return std::forward_like<Self>(self.data_[i]);

forward_like<Self>(x) reimposes Self's value category (and const-ness) onto x, regardless of how the subobject is qualified at the access site.

Impact

For char specifically, there's no observable runtime difference — moving a char is the same as copying it. So this is not a runtime bug in the example itself. The issue is pedagogical: this is a 'modern C++' reference doc that introduces deducing-this (C++23) but never mentions std::forward_like (also C++23, designed to be paired with it), and a reader who copies this pattern for a move-significant type — anything with a non-trivial T&&-qualified accessor, or where moving the element matters — will silently lose rvalue propagation. The doc's own comment overpromises what the code delivers.

Severity

Filing as nit — the example still correctly demonstrates the primary point (collapsing the const/non-const duplication) and char makes the move-vs-copy distinction invisible in practice. But the doc states its purpose is to teach the canonical modern idiom, and the canonical C++23 idiom here is std::forward_like. A one-line swap fixes it, or alternatively dropping 'x lvalue/rvalue' from the BEFORE comment to scope the claim correctly.