Improve Cpp::Destruct to return success/failure of wrapper generation
#653
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #653 +/- ##
==========================================
+ Coverage 78.80% 79.18% +0.38%
==========================================
Files 9 9
Lines 3854 3853 -1
==========================================
+ Hits 3037 3051 +14
+ Misses 817 802 -15
... and 1 file with indirect coverage changes
🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
lib/CppInterOp/CppInterOp.cpp
Outdated
| } | ||
|
|
||
| void Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class, | ||
| bool Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class, |
There was a problem hiding this comment.
warning: function 'Destruct' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
| bool Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class, | |
| static bool Destruct(compat::Interpreter& interp, TCppObject_t This, Decl* Class, |
|
The Windows Emscripten jobs will need to be checked manually as part of this PR. Despite them being green, they fail in the ci. The ci doesn't seem to be able to pick up the non zero exit code. I thought I'd managed to fix this bug, but it is still present. |
|
Can we add tests? |
|
|
||
| namespace Cpp { | ||
| void Destruct(compat::Interpreter& interp, TCppObject_t This, | ||
| bool Destruct(compat::Interpreter& interp, TCppObject_t This, |
There was a problem hiding this comment.
warning: function 'Destruct' can be made static or moved into an anonymous namespace to enforce internal linkage [misc-use-internal-linkage]
| bool Destruct(compat::Interpreter& interp, TCppObject_t This, | |
| static bool Destruct(compat::Interpreter& interp, TCppObject_t This, |
There was a problem hiding this comment.
Seems like a good suggestion.
There was a problem hiding this comment.
Agreed, but the C-API also uses this function and making it static would no longer make it visible
aaronj0
force-pushed
the
dtor-diagnostics
branch
2 times, most recently
from
9965a8e to
78c44c1
Compare
Done. The expected failure in the test does create a lot of verbosity when the wrapper is compiled. Any way to silence that? |
3 participants
No description provided.