-
Notifications
You must be signed in to change notification settings - Fork 35
[WIP] Things required to get Python Type-Hints to work with cppyy #708
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
base: main
Are you sure you want to change the base?
[WIP] Things required to get Python Type-Hints to work with cppyy #708
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #708 +/- ##
==========================================
- Coverage 79.04% 77.99% -1.05%
==========================================
Files 9 9
Lines 3879 3940 +61
==========================================
+ Hits 3066 3073 +7
- Misses 813 867 +54
🚀 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.
clang-tidy made some suggestions
There were too many comments to post at once. Showing the first 10 out of 14. Check the log or trigger a new build to see more.
|
|
||
| #include "clang/AST/Attrs.inc" | ||
| #include "clang/AST/Comment.h" | ||
| #include "clang/AST/CXXInheritance.h" |
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: #includes are not sorted properly [llvm-include-order]
| #include "clang/AST/CXXInheritance.h" | |
| #include "clang/AST/CXXInheritance.h" | |
| #include "clang/AST/Comment.h" |
lib/CppInterOp/CppInterOp.cpp
Outdated
| // std::deque avoids relocations and calling the dtor of InterpreterInfo. | ||
| static llvm::ManagedStatic<std::deque<InterpreterInfo>> sInterpreters; | ||
| static llvm::ManagedStatic<std::deque<std::shared_ptr<InterpreterInfo>>> | ||
| sInterpreters; |
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: variable 'sInterpreters' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
sInterpreters;
^
lib/CppInterOp/CppInterOp.cpp
Outdated
| sInterpreters; | ||
| static llvm::ManagedStatic< | ||
| std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>> | ||
| sInterpreterASTMap; |
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: variable 'sInterpreterASTMap' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
sInterpreterASTMap;
^
lib/CppInterOp/CppInterOp.cpp
Outdated
| static llvm::ManagedStatic< | ||
| std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>> | ||
| sInterpreterASTMap; | ||
| static std::mutex InterpreterStackLock; |
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: variable 'InterpreterStackLock' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
static std::mutex InterpreterStackLock;
^| } | ||
|
|
||
| bool IsTemplateClass(TCppScope_t handle) { | ||
| auto* D = (clang::Decl*)handle; |
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 use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* D = (clang::Decl*)handle;
^| std::string GetDocString(TCppScope_t scope) { | ||
| auto *D = static_cast<Decl*>(scope); | ||
| auto &AST = getASTContext(D); | ||
| const clang::RawComment *Comment = AST.getRawCommentForAnyRedecl(D); |
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: no header providing "clang::RawComment" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:42:
- #if CLANG_VERSION_MAJOR >= 19
+ #include <clang/AST/RawCommentList.h>
+ #if CLANG_VERSION_MAJOR >= 19
lib/CppInterOp/CppInterOp.cpp
Outdated
| } | ||
|
|
||
| auto* ND = Cpp_utils::Lookup::Named(&getSema(), name, Within); | ||
| auto* ND = Cpp_utils::Lookup::Named(&getSema(D), name, Within); |
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: no header providing "Cpp::utils::Lookup::Named" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:12:
+ #include "CppInterOpInterpreter.h"
lib/CppInterOp/CppInterOp.cpp
Outdated
| if (!ns) | ||
| return; | ||
|
|
||
| auto* D = (clang::Decl*)ns; |
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 use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* D = (clang::Decl*)ns;
^
lib/CppInterOp/CppInterOp.cpp
Outdated
| // the template function exists and >1 means overloads | ||
| bool ExistsFunctionTemplate(const std::string& name, TCppScope_t parent) { | ||
| DeclContext* Within = 0; | ||
| auto* D = (Decl*)parent; |
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 use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* D = (Decl*)parent;
^|
|
||
| bool IsStaticMethod(TCppConstFunction_t method) { | ||
| const auto* D = static_cast<const Decl*>(method); | ||
| if (auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) { |
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: 'auto *FTD' can be declared as 'const auto *FTD' [readability-qualified-auto]
| if (auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) { | |
| if (const auto *FTD = llvm::dyn_cast_or_null<FunctionTemplateDecl>(D)) { |
57d3266 to
955cdf7
Compare
955cdf7 to
5e53412
Compare
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
| if (!ns) | ||
| return; | ||
|
|
||
| auto* D = (clang::Decl*)ns; |
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 use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* D = (clang::Decl*)ns;
^
Description
Please include a summary of changes, motivation and context for this PR.
Fixes # (issue)
Type of change
Please tick all options which are relevant.
Testing
Please describe the test(s) that you added and ran to verify your changes.
Checklist