[cpplearner]

With modules, it is possible that a function template definition exists in a module but the definition is not yet loaded. In such a case, routine->defined is false, despite the existence of a definition. Previously, substitution failed if such a function template is called in a SFINAE context and the function template has a deduced return type. This is fixed in this PR.

See #242 for an example.

Fixes #242

Summary by CodeRabbit

• Bug Fixes
  • Fixed a C++20 modules issue where calling a function template with a deduced return type in a SFINAE context could incorrectly fail when the template definition had not yet been loaded.
  • Such calls can now be correctly recognized as valid during compile-time checks, including concept checks.

[cpplearner]

• Call has_pending_template_definition_from_module only if routine refers to a template instance.
• Drive by: simplify the neighboring friend_defined_in_instantiation check. The routine pointer was assigned routine_sym->variant.routine.ptr.

[daveedvdv-nvidia]

Thanks for taking this on!

I've created issue #453 from the discussion.

The fix looks fine to me (I made a note of one stylistic nit). To be complete we need a few more things:

1. A src/pending_changes/... entry explaining the improvement (referring to [GH #453])
2. One or more regressions tests
3. Rebase and collapse the commits into one, with a single-line commit message ending with [GH #453]

[daveedvdv-nvidia]

I'd slightly prefer a single if (tsym->defined || has_pending... ) ... statement here.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: edgcpp/compiler
• Branch: main
• File: .coderabbit.yaml
• Commit: cc40804
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 60ca2c9a-499f-4346-8a10-9f18f64af924

Commits

Reviewing files that changed from the base of the PR and between cc40804 and a137961.

Files selected for processing: 5

• src/exprutil.c
• src/pending_changes/gh-453.txt
• tests/tests/regressions/modules/gh-453-template-return-auto/A.ifc
• tests/tests/regressions/modules/gh-453-template-return-auto/A.ixx
• tests/tests/regressions/modules/gh-453-template-return-auto/test.mft.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The compiler now treats a pending module template definition as potentially instantiable when checking unresolved function templates with deduced return types in suppressed-diagnostic contexts. A C++20 modules regression test checks a requires-expression for an imported template.

Changes: Module template SFINAE

Layer / File(s)	Summary
Recognize pending module definitions src/exprutil.c	The suppressed-diagnostic check now treats a pending module template definition as evidence that a routine with an undeduced return type could be instantiated.
Test imported template constraint tests/tests/regressions/modules/gh-453-template-return-auto/*, src/pending_changes/gh-453.txt	The module exports a function template with a deduced return type. The regression test checks that a requires-expression accepts the call, and the changelog records the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related issues

• edgcpp/compiler#453 — Describes the same failure for a module-imported function template with a deduced return type in a requires-expression or SFINAE context.

Suggested labels: bug

Suggested reviewers: daveedvdv-nvidia, wchilders-nvidia

Merge Risk: Minimal · up to a1379

No actionable merge-blocking risk is established by this change.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and specifically summarizes the main change: fixing SFINAE for function templates with deduced return types and pending definitions.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[cpplearner]

The fix looks fine to me (I made a note of one stylistic nit). To be complete we need a few more things:

1. A src/pending_changes/... entry explaining the improvement (referring to [GH Incorrect constraint check for call to module-imported function template with deduced return type #453])
2. One or more regressions tests
3. Rebase and collapse the commits into one, with a single-line commit message ending with [GH Incorrect constraint check for call to module-imported function template with deduced return type #453]

Done.

The test depends on an ifc file generated by MSVC. (EDG cannot generate it. It emits an internal error in "ifc_modules_write.c". Repro: https://godbolt.org/z/9ETPa6re1) I included the ifc file in the test as well as the ixx source file. Please let me know if there's a better way of structuring the module test.

[edgcpp-bot]

AcknowlEDG review results updated for a1379613b347:

• Bench - C (unchanged)
• Bench - C++ (unchanged)
• Test - Changes (new)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[wchilders-nvidia]

We took all of the MSVC IFC files out of the test when opening up the repo because of legal concerns with distributing them. I'm not sure those have been resolved, so I think I'd unfortunately prefer to pull this as an E-IFC test that would fail (because the writing support is not currently present).

I have some old patches I should push up somewhere that were WIP expansions of friend and template support for E-IFC.