- 
                Notifications
    You must be signed in to change notification settings 
- Fork 35
Refactoring the API for multiple interpreter instance support #338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-tidy made some suggestions
5c64f77    to
    0cf2bf1      
    Compare
  
    | void Destruct(TCppObject_t This, TCppScope_t scope, bool withFree /*=true*/) { | ||
| Decl* Class = (Decl*)scope; | ||
| if (auto wrapper = make_dtor_wrapper(getInterp(), Class)) { | ||
| TCppObject_t Construct(TCppScope_t scope, void* arena /*=nullptr*/) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also removed the Impl suffix.
| Codecov ReportAttention: Patch coverage is  
 
 Additional details and impacted files@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
+ Coverage   74.29%   74.43%   +0.14%     
==========================================
  Files           8        8              
  Lines        3190     3204      +14     
==========================================
+ Hits         2370     2385      +15     
+ Misses        820      819       -1     
 ... and 1 file with indirect coverage changes 
 | 
fedb10d    to
    92ab4c7      
    Compare
  
    9d76fcc    to
    4ec28eb      
    Compare
  
    | clang-tidy review says "All clean, LGTM! 👍" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Thank you, @Gnimuc!
| Can you squash the commits to have clear history? | 
also fix argument type typos and c-style casts
4ec28eb    to
    9acb9e8      
    Compare
  
    | 
 Done! | 
I feel like we also need to split the implementation and use an internal shared header for interfaces.