Repository navigation
Keep C++11 constexpr templates usable after non-return statement warning [GH #356] - #407
Conversation
|
Caution Review failedThe pull request is closed. No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Condition constexpr exclusion on diagnostic severity src/statements.c |
The selected constexpr-body diagnostic code is reported, and declared constexpr or consteval functions are ruled out only when the diagnostic is an effective error. |
Test warning cases and record behavior tests/tests/regressions/gh-356.sft.cpp, tests/tests/regressions/.gh-356.rto/*, src/pending_changes/gh-356-cpp14-constexpr-func-in-cpp11.chng.txt |
The C++11 regression test checks constexpr construction, template argument evaluation, and function evaluation. Expected outputs record the warnings, and the changelog describes the behavior. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- edgcpp/compiler#386: Both PRs address GH
#356instatement()and constexpr evaluation of C++11 templates.
Suggested labels: bug
Suggested reviewers: nv-cmeerw
Merge Risk: 
b8bbe
-
The change is ready to merge; normal execution of the added regression test remains appropriate validation.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the primary change: C++11 constexpr templates stay usable after a non-return statement warning, which matches the statements.c change and the GH #356changelog, test, and regression fi…Linked Issues check Issue #356requires that a C++11 constexpr constructor or function template, accepted with a warning for a C++14-style statement in GNU 120000+ or Clang 30300+ mode, remain usable in constant expressi…Out of Scope Changes check All changes support issue #356. The src/statements.c hunk implements the required conditional constexpr_ruled_out logic. It also moves the err_code selection into a local variable so that the same cod…Full details: Title check
-
Explanation
The title names the primary change: C++11 constexpr templates stay usable after a non-return statement warning, which matches the statements.c change and the GH
#356changelog, test, and regression files. It also names the issue, so a reviewer scanning history can identify the fix.
Full details: Linked Issues check
-
Explanation
Issue
#356requires that a C++11 constexpr constructor or function template, accepted with a warning for a C++14-style statement in GNU 120000+ or Clang 30300+ mode, remain usable in constant expressions. The diff in src/statements.c (statement()) changes the constexpr-body path. It now sets constexpr_ruled_out = TRUE for a declared constexpr or consteval routine only when is_effective_error(err_code, in_constexpr_body_sev, &start_pos) is TRUE. When the diagnostic is downgraded to es_warning, the routine stays eligible for constant evaluation. The constructor case is covered because err_code is selected with special_kind_is(current_rp, sfk_constructor). The added regression test tests/tests/regressions/gh-356.sft.cpp uses the issue's constructor-template pattern (A<int> a(5); static_assert(a.x == 5, "");) with the GNU 150000 and Clang 190000 options. It also covers a constexpr function template (f(41) == 42) and a template-argument use. Non-declared constexpr routines keep the previous unconditional ruled-out behavior through the else branch. The issue's objective is implemented by the code change. Test results were not executed in this review. The is_effective_error declaration was not inspected; its use mirrors check_void_return_okay as stated in the PR description.
Full details: Out of Scope Changes check
-
Explanation
All changes support issue
#356. The src/statements.c hunk implements the required conditional constexpr_ruled_out logic. It also moves the err_code selection into a local variable so that the same code is reused for both pos_diagnostic and is_effective_error. The new regression test and its .rto expected-output files exercise the reported scenario. The changelog entry src/pending_changes/gh-356-cpp14-constexpr-func-in-cpp11.chng.txt documents the fix. No unrelated source or behavior change was found in the reviewed diff. The one behavioral side effect is that uninstantiated prototype or template routines whose diagnostic is downgraded to a warning are no longer ruled out. This follows directly from the issue's warning-to-eligibility requirement.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
AcknowlEDG review results updated for
|
|
AcknowlEDG review results updated for
CI success. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
b8bbe31 to
e5cb7da
Compare
Fixes #356.
When a statement that is not permitted in a C++11
constexprfunction or constructor is accepted with a warning (for example, in a function or constructor template in some GNU or Clang C++11 modes),statement()insrc/statements.cpreviously still setconstexpr_ruled_out = TRUEunconditionally. As a result, the routine was marked as unusable in constant expressions.Only set
constexpr_ruled_out = TRUEfor an explicitlyconstexpr/constevalroutine whenis_effective_error(err_code, in_constexpr_body_sev, &start_pos)isTRUE(matchingcheck_void_return_okay), so routines accepted with a warning remain eligible for constant evaluation.Summary by CodeRabbit
constexprtemplate or constructor contains a statement that triggers a warning. Such expressions can now be used in constant evaluation, including in static assertions and template arguments.constexprfunction or constructor from being used in constant evaluation.