Repository navigation
Conversation
…nMatch and capture their diagnostics [upstream] The body of the best match was instantiated with all diagnostics suppressed, and the match only rejected if the specialization itself was marked invalid. Errors which leave it valid, such as a failing static_assert or an error in a nested instantiation, were therefore ignored: the former resulted in running code which should not have compiled, the latter in a failure at link time for the missing nested definition, without the actual compiler error being reported anywhere. Now the match is rejected if any error occurred during the instantiation (using a DiagnosticErrorTrap), and the specialization marked invalid so that later lookups reject it as well. The diagnostics are rendered into a string as clang would print them, rather than suppressed, and made available through the new GetLastOverloadMatchDiagnostics, so that callers can report them where appropriate while probing for a valid match stays quiet. Upstream: compiler-research/CppInterOp#1179 (the same change on top of the reworked BestOverloadFunctionMatch on CppInterOp main, which also includes the earlier ROOT patch "Reject ill-formed instantiations in BestOverloadFunctionMatch" that this builds on). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Include the compiler errors from the instantiation of a template called from python in the "Failed to instantiate" error, and report compiler warnings from a successful instantiation as a python UserWarning, using the diagnostics captured by CppInterOp's overload resolution. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…test19_templated_operator_add Add tests for the reporting of errors and warnings from template instantiations, including the regression test for root-project#11854, and rejecting instantiations with a failing static_assert. The operator+ in test19_templated_operator_add accesses the private member of other specializations, which is ill-formed (and rejected by gcc and clang) and so is now rejected also when instantiated from python, so add the needed friend declaration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hi @bendavid , Thanks for this contribution! FWIW, note that I had started to implement your original PR, splitting it by the 4 categories I had highlighted at #12449 (comment) The first part of this is #20946, which immediately failed on many regards and was apparently backwards incompatible (at least on the surface) from the point of view of cling, not even touching the Python bindings. In particular, see this issue I had opened as a followup to the failures in my PR #20952. Also, as I noted in my PR, I was not able to create a clear, C++-only reproducer to pinpoint how the changes actually fix something without breaking anything else. I believe all of this should be addressed before considering the rest of this PR. Unless this PR is adopting a completely different approach w.r.t. before, such that cling is not touched and all my premise loses relevance. |
|
I just took a look at the code changes and indeed they do not touch cling anymore, which is both relieving and surprising. In any case, I would like to see if #20952 is now fixed to close that other effort without running the risk of leaving behind us untracked issues. |
This Pull request:
Reports the compiler diagnostics from instantiating templates called from Python with the cppjit bindings, and rejects ill-formed instantiations instead of silently running them. This replaces #12449 for the CppInterOp/cppjit-based bindings, following the split proposed at the end of that PR's discussion (CppInterOp facility first, then its use in the bindings).
Note on the vendored CppInterOp: this PR edits the copy in
interpreter/CppInterOpdirectly. It is therefore expected to fail thecppinterop-diffcheck and not meant to be merged in this form. It's opened to make the complete change visible and testable for the discussion. The CppInterOp part is proposed upstream as compiler-research/CppInterOp#1179, rebuilt on the currentmainthere. Once that is settled, the vendored copy would come in through the usualROOT-cppjitupdate, and this PR would reduce to the cppjit and test commits.For the same reason, the
cppjit-diffcheck is expected to fail:bindings/pyroot/cppjitis likewise a vendored copy of compiler-research/cppjit, and the cppjit changes here would go upstream there as well. Theclang-formatcheck's only suggestion is to reformat the new entry in the vendoredCppInterOp.td(a TableGen file) in ROOT's C++ style. That isn't applied, to keep the entry consistent with the rest of the file and with upstream.Changes or fixes:
[cppinterop](vendored, upstream as Reject ill-formed template bodies in BestOverloadFunctionMatch and capture their diagnostics compiler-research/CppInterOp#1179):BestOverloadFunctionMatchinstantiates the body of the best match, the match is now rejected if any error occurred, not only if the specialization was marked invalid. Errors such as a failingstatic_assertor an error in a nested instantiation leave it valid. Previously those cases either ran code that shouldn't have compiled, or failed at link time without the compiler error being shown.GetLastOverloadMatchDiagnostics().[cppjit]:TypeErrorfor a failed instantiation, indented under the corresponding "Failed to instantiate" line.UserWarning.[python]tests:template_instantiation_diagnostics.py, including the reproducer from Bad error handling with pyroot template instantiations #11854 and a failingstatic_assert.test19_templated_operator_addgets the friend declaration it needs: itsoperator+accessed private members of other specializations, which is ill-formed (gcc and clang reject it) and is now also rejected from Python.Example (from #11854): instead of a link-time failure with no explanation, you now get:
Checklist:
masterwith these commits (gminimal,pyroot,testing, C++20, Arch Linux with GCC). All 65cppjit|pyrootctests pass, including the newpyroot-template-instantiation-diagnosticsandcppjit-test-templates. The CppInterOp unit tests for the upstream version are described in Reject ill-formed template bodies in BestOverloadFunctionMatch and capture their diagnostics compiler-research/CppInterOp#1179.This PR fixes #11854
🤖 Generated with Claude Code