-
Notifications
You must be signed in to change notification settings - Fork 30
Fix bugs related to printing types #661
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
Fix bugs related to printing types #661
Conversation
@@ -517,18 +517,28 @@ std::string GetCompleteName(TCppType_t klass) { | |||
auto& C = getSema().getASTContext(); | |||
auto* D = (Decl*)klass; | |||
|
|||
PrintingPolicy Policy = C.getPrintingPolicy(); |
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.
I wonder if it would be a good idea to improve the to-string printing consistency by modifying the default printing policy at CreateIntepreter
time?
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.
If the intention is to; not keep modifying the PrintingPolicy
, it may not be possible. We need different amounts of information at different times, using different APIs. Where we will have to modify the PrintingPolicy
.
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.
Can we open an issue to track this idea?
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.
Opened an issue at #662
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #661 +/- ##
==========================================
+ Coverage 79.18% 79.25% +0.06%
==========================================
Files 9 9
Lines 3853 3866 +13
==========================================
+ Hits 3051 3064 +13
Misses 802 802
🚀 New features to boost your workflow:
|
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!
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
@@ -2102,6 +2111,20 @@ TEST(FunctionReflectionTest, GetFunctionArgDefault) { | |||
EXPECT_EQ(Cpp::GetFunctionArgDefault(Decls[4], 1), ""); | |||
EXPECT_EQ(Cpp::GetFunctionArgDefault(Decls[4], 2), "\'a\'"); | |||
EXPECT_EQ(Cpp::GetFunctionArgDefault(Decls[4], 3), "0."); | |||
|
|||
ASTContext& C = Interp->getCI()->getASTContext(); | |||
Cpp::TemplateArgInfo template_args[1] = {C.IntTy.getAsOpaquePtr()}; |
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.
warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]
Cpp::TemplateArgInfo template_args[1] = {C.IntTy.getAsOpaquePtr()};
^
ASTContext& C = Interp->getCI()->getASTContext(); | ||
Cpp::TemplateArgInfo template_args[1] = {C.IntTy.getAsOpaquePtr()}; | ||
Cpp::TCppScope_t my_struct = | ||
Cpp::InstantiateTemplate(Decls[6], template_args, 1); |
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.
warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
Cpp::InstantiateTemplate(Decls[6], template_args, 1);
^
EXPECT_EQ(Cpp::GetCompleteName(nullptr), "<unnamed>"); | ||
|
||
ASTContext& C = Interp->getCI()->getASTContext(); | ||
Cpp::TemplateArgInfo template_args[2] = {C.IntTy.getAsOpaquePtr(), |
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.
warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]
Cpp::TemplateArgInfo template_args[2] = {C.IntTy.getAsOpaquePtr(),
^
ASTContext& C = Interp->getCI()->getASTContext(); | ||
Cpp::TemplateArgInfo template_args[2] = {C.IntTy.getAsOpaquePtr(), | ||
C.DoubleTy.getAsOpaquePtr()}; | ||
Cpp::TCppScope_t fn = Cpp::InstantiateTemplate(Decls[11], template_args, 2); |
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.
warning: do not implicitly decay an array into a pointer; consider using gsl::array_view or an explicit cast instead [cppcoreguidelines-pro-bounds-array-to-pointer-decay]
Cpp::TCppScope_t fn = Cpp::InstantiateTemplate(Decls[11], template_args, 2);
^
This reverts commit f65f80a.
Description
Fixes # (issue)
Helps fix up to 5 tests in cppyy.
Type of change
Please tick all options which are relevant.
Testing
Checklist