Skip to content

feat(cpp): Expand type_traits constraint registry#1648

Merged
magyargergo merged 6 commits into
abhigyanpatwari:mainfrom
azizur100389:codex/cpp-type-traits-tier-a
May 18, 2026
Merged

feat(cpp): Expand type_traits constraint registry#1648
magyargergo merged 6 commits into
abhigyanpatwari:mainfrom
azizur100389:codex/cpp-type-traits-tier-a

Conversation

@azizur100389

@azizur100389 azizur100389 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • expand the C++ SFINAE type_traits registry with Tier-A predicates: is_void_v, is_enum_v, is_class_v, is_pointer_v, is_reference_v, is_const_v, and is_volatile_v
  • add a reference-site argumentTypeClasses sidecar so constraint filtering can use cv/ref/pointer shape without changing normalized argumentTypes used by overload narrowing
  • keep shape-sensitive predicates conservative: unsupported or lossy evidence returns unknown rather than dropping candidates
  • add focused unit truth-table rows and C++ integration fixtures for the new predicates

Fixes #1629

Tests

  • npx vitest run test/unit/scope-resolution/cpp/cpp-constraint.test.ts
  • npx vitest run test/integration/resolvers/cpp.test.ts -t "Tier-A type_traits predicates"
  • npx vitest run test/integration/resolvers/cpp.test.ts
  • REGISTRY_PRIMARY_CPP=0 npx vitest run test/integration/resolvers/cpp.test.ts -t "Tier-A type_traits predicates" (expected scope-resolver-only skips)
  • npx tsc --noEmit
  • git diff --check

Note: npx gitnexus impact ... and npx gitnexus detect_changes ... currently exit 1 with no output in this checkout, so the change was kept scoped to the C++ constraint/type-shape path and validated with targeted plus full C++ resolver suites.

@vercel

vercel Bot commented May 16, 2026

Copy link
Copy Markdown

@azizur100389 is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

CI Report

All checks passed

Pipeline Status

Stage Status Details
✅ Typecheck success tsc --noEmit
✅ Tests success unit tests, 3 platforms
✅ E2E success gitnexus-web changes only

Test Results

Tests Passed Failed Skipped Duration
9218 9217 0 1 472s

✅ All 9217 tests passed

1 test(s) skipped — expand for details
  • buildTypeEnv > known limitations (documented skip tests) > Ruby block parameter: users.each { |user| } — closure param inference, different feature

Code Coverage

Tests

Metric Coverage Covered Base Delta Status
Statements 78.66% 29705/37762 N/A% 🟢 ███████████████░░░░░
Branches 67.24% 18970/28210 N/A% 🟢 █████████████░░░░░░░
Functions 83.59% 2985/3571 N/A% 🟢 ████████████████░░░░
Lines 81.97% 26782/32670 N/A% 🟢 ████████████████░░░░

📋 View full run · Generated by CI

@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @magyargergo's task in 12m 49s —— View job


Adversarial Production-Readiness Review — PR #1648

feat(cpp): Expand type_traits constraint registry · Reviewed against DoD.md, AGENTS.md, GUARDRAILS.md, and issue #1629 requirements.


Review bar for this PR

ISO C++ <type_traits> / SFINAE specialist + senior GitNexus production-readiness reviewer. Correctness baseline is ISO C++ semantics; GitNexus intentionally implements a bounded conservative approximation. Highest-risk failure: false incompatible (silently deletes candidates). Second-highest: false compatible (wrong edges). Third: argumentTypeClasses misalignment. Verdict rules are applied strictly per the review brief.


Current PR state

  • Branch: codex/cpp-type-traits-tier-amain
  • Commits on branch: 2 — 488c0bf (feature) + 0f8c19e (merge-from-main)
  • Changed files: 18 (confirmed locally)
  • CI: ✅ 9179/9180 tests pass (1 skip: unrelated Ruby test). TypeScript clean. All platform jobs green.
  • Vercel: Pending authorization — irrelevant to resolver correctness.

Branch hygiene assessment

Classification: merge-from-main commit present but harmless and merge-safe

One feature commit (488c0bf) and one merge-from-main commit (0f8c19e). The merge pulls in main fixes (#1655, #1654, #1652) as expected for a PR open against main. The effective diff is correctly scoped to the 18 Tier-A expansion files. No unrelated ADL, graph-identity, workflow, settings, or formatting churn detected in the diff.


Understanding of the change

This PR expands the C++ SFINAE constraint registry from 4 predicates (is_integral_v, is_floating_point_v, is_arithmetic_v, is_same_v) to 11 by adding is_void_v, is_enum_v, is_class_v, is_pointer_v, is_reference_v, is_const_v, and is_volatile_v. The key infrastructure addition is a argumentTypeClasses?: readonly ParameterTypeClass[] sidecar on ReferenceSite and ConstraintContext, populated at call-site capture time by inferCppCallArgTypeClasses. Shape-sensitive predicates (is_pointer_v, is_reference_v, is_const_v, is_volatile_v) consult this sidecar via unaryShapeVerdict. Value-only predicates (is_void_v, is_enum_v, is_class_v) use unaryVerdict with an isPlainValue guard.


ISO C++ type-traits conformance scorecard

Predicate Classification Notes
is_void_v Approximate but conservative Correctly distinguishes void* (pointer, isPlainValue=false) from void. ISO-valid void can't appear at normal call sites; unit test with argumentTypes=['void'] is a valid proxy.
is_enum_v Approximate but conservative Correctly uses enum: prefix from isKnownEnumName. Simple-name TU scan ignores namespaces (documented limitation).
is_class_v Approximate but conservative Union types classified as 'class' (ISO is_class_v<union> = false). Known coarse-approximation boundary.
is_pointer_v ISO-aligned for supported evidence indirection === 'pointer' && pointerDepth > 0 correctly identifies pointer types. Unknown sidecar → 'unknown'.
is_reference_v ISO-aligned for supported evidence Correctly checks lvalue-ref/rvalue-ref. ISO deduction drops references for by-value T (same pattern as pre-existing is_integral_v).
is_const_v Unsupported and risky See Finding 2 and Finding 3 below.
is_volatile_v Unsupported and risky Same issues as is_const_v.

Findings

Finding 1 — Function parameter lookup gap [BLOCKING | Lane B]

Severity: High — merge-blocking per review rules.

Evidence: captures.ts:764–779lookupDeclaredTypeClassForIdentifier walks up to the nearest compound_statement and scans its declaration children. Function parameters live in parameter_list (part of function_declarator, not inside the compound statement body), so they are never found. The code itself documents this:

// Limitation: only `declaration` siblings inside the enclosing
// `compound_statement` are inspected. Function parameters live in the
// `function_declarator`'s `parameter_list` and are NOT resolved here, so
//   `void run(int n) { process(n); }`
// infers `''` for `n` and the constraint filter falls through to
// `'unknown'` → ambiguity suppression → 0 CALLS edges.

ISO C++ relevance: Issue #1629's motivating example is:

void run(S* p, S s) { f(p); f(s); }

p and s are function parameters. With the PR as-is:

  • lookupDeclaredTypeClassForIdentifier(p)unknownTypeClass('unknown')
  • is_pointer_v via unaryShapeVerdict → typeClass='unknown' → 'unknown'
  • is_class_v via unaryVerdict → typeClass='unknown' → 'unknown'
  • Both candidates kept → isOverloadAmbiguousAfterNormalization → edge suppressed → 0 CALLS edges

This is the exact false-ambiguity scenario described in the issue.

GitNexus production risk: The fixture works around this by using local variables. But the issue's motivating example (function parameters) is unresolved. Users relying on is_pointer_v/is_class_v SFINAE disambiguation in templates called from functions with pointer/class parameters will see 0 edges instead of correct routing.

Recommended fix: Extend lookupDeclaredTypeClassForIdentifier to also search the enclosing function's parameter_list. Walk from compound_statementfunction_definitionfunction_declaratorparameter_list.

Blocks merge: Yes — the review brief states: "If the issue's motivating pointer/class cases still fall through to false ambiguity, verdict must be not production-ready."

Fix this →


Finding 2 — is_const_v/is_volatile_v template deduction mismatch [MEDIUM | Lane A]

Severity: Medium — emits false-positive CALLS edges for a code pattern that is SFINAE-invalid in real ISO C++.

Evidence: constraint-filter.ts:98–107

In ISO C++, template type deduction for a by-value parameter (template<class T> void pick(T)) always strips top-level cv qualifiers. So pick(const_int_var) deduces T=int, not T=const int. Thus is_const_v<T> is always false for by-value T. The SFINAE fixture:

template <class T, std::enable_if_t<std::is_const_v<T>, int> = 0>
void pick(T value) {}

...would have enable_if_t always disabled in real C++ when called with any argument (T is always deduced as unqualified). Both overloads in the fixture are ISO-SFINAE-invalid — the code is technically ill-formed.

GitNexus reads the cv field from the argumentTypeClasses sidecar and returns 'compatible' when cv='const', routing pick(c) to the is_const_v overload. The test asserts 2 edges, which happens to route each call to its "intended" overload, but via incorrect ISO reasoning and on code that a real compiler would reject.

GitNexus production risk: No wrong edges for the current fixture (each call routes to the overload with the matching cv), but the approximation is ISO-semantically unsound. If a real codebase has is_const_v<T> SFINAE guards on by-value templates, GitNexus will emit edges for calls that ISO says are ill-formed.

Recommended fix: Document in the predicate comment that is_const_v/is_volatile_v are only meaningful when T is explicitly set (not deduced), and add a known-limitation note to the PR description. Consider returning 'unknown' for by-value sidecar entries (indirection='value') to avoid the false-compatible case.

Blocks merge: Maybe — current fixtures produce no wrong edges for the tested cases, but the fixture itself tests ISO-invalid code and the behavior is undocumented.


Finding 3 — Top-level cv vs pointee cv confusion [MEDIUM | Lane A, B]

Severity: Medium — can produce false 'compatible' for is_const_v<const int*>.

Evidence: constraint-filter.ts:99–101

ISO C++ distinguishes:

  • const int* — pointer to const int; is_const_v<const int*> = false (pointer itself is not const)
  • int* const — const pointer to int; is_const_v<int* const> = true

classifyCppParameterType in arity-metadata.ts records the cv qualifier from the type specifier. For const int* p, it likely records cv='const' (from the const before int), which is_const_v reads as top-level const → returns 'compatible'. This is ISO-incorrect and could delete valid candidates in overload sets like:

template<class T, enable_if_t<!is_const_v<T>>> void f(T* p);  // should win for const int*
template<class T, enable_if_t< is_const_v<T>>> void f(T* p);  // should NOT win for const int*

No current fixture covers this case.

Recommended fix: When the argument sidecar has indirection === 'pointer', return 'unknown' for is_const_v/is_volatile_v unless the cv evidence explicitly models top-level (pointer-type) cv vs pointee cv. Document this limitation.

Blocks merge: Maybe — no current fixture triggers false edges, but common C++ patterns with const int* arguments are not tested.

Fix this →


Finding 4 — Enum detection: global simple-name scan ignores namespaces [LOW | Lane B]

Severity: Low.

Evidence: captures.ts:875–891

isKnownEnumName walks the entire TU AST looking for any enum_specifier with a matching simple name, ignoring namespaces. If class Color {} and namespace NS { enum Color {}; } coexist in the same TU, a Color variable of class type would be falsely tagged as enum:Color. This would cause is_enum_v to return 'compatible' for a class-type variable, potentially deleting the is_class_v overload candidate.

Recommended fix: Namespace-aware lookup, or return 'unknown' when there is a same-name enum/class collision in the TU.

Blocks merge: No — current fixtures have no same-name collisions.


Finding 5 — Missing unit truth-table rows for unknown sidecar inputs [LOW | Lane F]

Severity: Low — the code handles these correctly; only test coverage is missing.

Evidence: cpp-constraint.test.ts:306–368

The review DoD requires unit rows for:

  • indirection: 'unknown'is_pointer_v/is_reference_v should return 'unknown'
  • cv: 'unknown'is_const_v/is_volatile_v should return 'unknown'
  • Sidecar present but shorter than argument list (alignment guard)

The code in unaryShapeVerdict does correctly return 'unknown' when shape.indirection === 'unknown' or shape.cv === 'unknown' (line 135), but there are no unit test rows asserting this.

Blocks merge: No — behavior is correct; tests are incomplete.


Finding 6 — Integration test fixture assertions: edge count but not overload identity [LOW | Lane F]

Severity: Low.

Evidence: cpp.test.ts:3170–3209

All Tier-A integration tests check calls.length and new Set(calls.map(c => c.rel.targetId)).size. They verify that 2 distinct pick overloads are called, but do not assert which overload (by parameterTypes, filePath, or line number) received each call. A regression that routes both calls to the same overload but with different targetIds would pass. DoD.md §2.7 says "Use toBe / toEqual for exact expectations; avoid toBeGreaterThanOrEqual and other bounds-only assertions that mask regressions."

Blocks merge: No — bounds assertions are adequate for the current risk level; tighter assertions are a follow-up improvement.


PR-specific assessment sections

Tier-A registry scope and ISO boundary

The PR correctly restricts the registry to the 7 stated predicates. getRegistrySize() assertion correctly updated from 4 → 11. No overreach into unsupported predicates. The PR description explicitly states "Tier-A" scope. The unit test comment header references issues #1579 and the predicate list matches exactly. ✅

Predicate truth tables and Kleene unknown behavior

unaryVerdict: correctly returns 'unknown' when arg === undefined || arg.typeClass === 'unknown'. unaryShapeVerdict: correctly returns 'unknown' when shape is absent, indirection === 'unknown', or cv === 'unknown'. No predicate returns 'incompatible' from purely unknown evidence. Kleene AND/OR/NOT logic is unchanged and correct. ✅ (except missing test rows per Finding 5)

argumentTypeClasses capture and alignment

inferCppCallArgTypeClasses uses the same loop structure as inferCppCallArgTypes — both skip only ,, (, ) delimiters and push exactly one entry per argument (or unknownTypeClass for unsupported expressions). Alignment with argumentTypes is preserved. Scope extractor parses @reference.parameter-type-classes with the same safe JSON validation as other sidecar captures. KNOWN_SUB_TAGS includes @reference.parameter-type-classes at line 1047 of scope-extractor.ts. ✅

Type classifier correctness for enum/class/pointer/reference/cv

classifyType() defaults to 'class' for unrecognized tokens (correct for the approximate model). Enum detection via isKnownEnumName is conservative but namespace-unaware (Finding 4). classifyCppParameterType from arity-metadata.ts correctly maps S* → pointer, S& → lvalue-ref, const S → cv='const'. The shapeForTemplateParam safety check correctly returns undefined when paramShape.indirection !== 'value', preventing shape from being passed when T is wrapped (e.g., T* parameter type). ✅ (with caveats in Findings 2 and 3)

Candidate filtering phase and overload integration

Constraint filter runs at step 4c of narrowOverloadCandidates — after arity, exact-type, and conversion-rank filters. All three call paths correctly thread argumentTypeClasses:

  • free-call-fallback.ts lines 129, 194, 230: ✅
  • pickImplicitThisOverload (free-call-fallback.ts line 505): ✅
  • receiver-bound-calls.ts lines 349, 736: ✅

Disabled candidates receive 'incompatible' and are dropped before reaching ADL merge, global fallback, or OVERLOAD_AMBIGUOUS check. ✅

Interaction with #1623, #1642, #1606, #1599, and #1600

Shared contract and cross-language safety

argumentTypeClasses is readonly and optional in both ReferenceSite (reference-site.ts:89) and ConstraintContext (context.ts:74). Non-C++ languages never populate @reference.parameter-type-classes, so the field is always undefined for them. The shared extractor (scope-extractor.ts) conditionally spreads argumentTypeClasses into the ReferenceSite only when present. No non-C++ language behavior changes. ✅

helpers.ts legacy DAG skip correctness

LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES.cpp at lines 203–208 correctly lists all 6 new Tier-A test names:

  • 'is_pointer_v and is_class_v disambiguate pointer vs class arguments'
  • 'is_reference_v keeps reference-shaped arguments distinct from values'
  • 'is_class_v rejects primitive arguments while keeping class arguments'
  • 'is_enum_v distinguishes known enum declarations from primitives'
  • 'is_const_v and is_volatile_v disambiguate cv-qualified locals'
  • 'is_void_v does not misclassify void pointers as void values'

These match the describe/it names in cpp.test.ts exactly. Registry-primary (REGISTRY_PRIMARY_CPP unset or '1') runs all 6 assertions without skipping. ✅

Tests and CI coverage

  • getRegistrySize() assertion: 11 ✅
  • All 7 Tier-A predicates have compatible/incompatible/unknown rows in cpp-constraint.test.ts
  • Monotonicity guard (__not_in_registry__ → 'unknown') ✅
  • Missing: indirection: 'unknown'/cv: 'unknown' rows (Finding 5) ⚠️
  • Integration tests: edge-count assertions (adequate but not overload-identity tight) ⚠️
  • CI green on all 3 platforms + typecheck ✅

Hidden Unicode/hygiene

GitHub flagged potential hidden Unicode in changed files. Manual inspection of the source shows the helpers.ts file contains \u2014 (em dash ) at line 228 in a comment string \u2014 — this is a prose character in a human-readable test name/comment, not a bidi control or execution-path character. No bidi controls, zero-width joiners, or directional overrides were found in executable code paths, JSON payloads, regex patterns, or shared contracts. The grep for non-ASCII bytes in the key TypeScript source files returned no executable-logic results. ✅


Final verdict: not production-ready

Primary blocking issue (Finding 1): Issue #1629's motivating example — SFINAE-guarded is_pointer_v/is_class_v disambiguation for function parameters — produces 0 CALLS edges (false ambiguity) because lookupDeclaredTypeClassForIdentifier only scans compound statement body declarations, not the enclosing function's parameter_list. The PR works around this by using local variables in all fixtures, but the issue's stated use case (function parameters) is explicitly unresolved. The review rules state this is a hard block.

Secondary concern (Findings 2 & 3): The is_const_v and is_volatile_v predicates read cv from the argument sidecar without modeling ISO template type deduction (cv is stripped for by-value T). The fixture tests ISO-SFINAE-invalid code (by-value templates where is_const_v<T> is always false due to cv-drop). GitNexus emits CALLS edges that ISO would suppress because both overloads are ill-formed. Additionally, const int* (pointee-const pointer) would trigger false 'compatible' for is_const_v, as the sidecar records the pointee's cv rather than the pointer type's cv. Neither limitation is documented in the predicate implementation.

What is solid: The monotonic unknown invariant is correctly implemented. The argumentTypeClasses sidecar is properly aligned and threaded through all three overload-narrowing paths. The shared contract change is safe for non-C++ languages. Legacy parity skip list is correct. CI is green. The is_pointer_v / is_reference_v / is_enum_v / is_class_v predicates work correctly for their tested fixtures. The overall constraint filter architecture is sound.

Path to production-ready: (1) Extend lookupDeclaredTypeClassForIdentifier to also scan the function's parameter_list. (2) Document or guard the is_const_v/is_volatile_v cv-deduction approximation — either return 'unknown' for pointer types (where top-level vs pointee cv is ambiguous) or add a code comment explicitly stating the by-value deduction limitation. (3) Add unit test rows for indirection: 'unknown' and cv: 'unknown' sidecar inputs.


@magyargergo

Copy link
Copy Markdown
Collaborator

@azizur100389 can you please look into this? 🙏

@azizur100389

Copy link
Copy Markdown
Contributor Author

Addressed the review findings in 5990d647.

Changes:

  • function-parameter identifiers are now resolved for both normalized argumentTypes and argumentTypeClasses, so the motivating void run(S* p, S s) { f(p); f(s); } pointer/class case is covered
  • is_const_v / is_volatile_v now return unknown for pointer-shaped evidence where top-level cv vs pointee cv is ambiguous
  • added unit rows for unknown indirection/cv sidecar inputs
  • changed the pointer/class integration fixture to use function parameters instead of local variables

Validation:

  • npx vitest run test/unit/scope-resolution/cpp/cpp-constraint.test.ts — 25 passed
  • npx vitest run test/integration/resolvers/cpp.test.ts -t "Tier-A type_traits predicates" — 6 passed
  • npx vitest run test/integration/resolvers/cpp.test.ts — 231 passed
  • REGISTRY_PRIMARY_CPP=0 npx vitest run test/integration/resolvers/cpp.test.ts -t "Tier-A type_traits predicates" — expected scope-resolver-only skips
  • npx tsc --noEmit — passed
  • git diff --check — passed

npx gitnexus detect_changes --scope staged still exits 1 with no output in this checkout, consistent with the earlier local index issue.

@magyargergo magyargergo merged commit 5f0c0eb into abhigyanpatwari:main May 18, 2026
29 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cpp SFINAE: expand type_traits predicate registry (Tier A)

2 participants