Mutex lock (implementation base on ASTContext::getAllocator::identifyObject)
#704
Conversation
We need to identify which interpreter a Decl belongs to, when using multiple interpreter. We do it by checking which `clang::ASTContext` the `clang::Decl` belongs We maintain a map: `clang::ASTContext -> Cpp::InterpreterInfo`. Using this map, be identify the correct interpreter. There are 2 usecases for this: 1. We can now lock the correct interpreter making it thread safe. 2. User of `libCppInterOp` need not set the correct active interpreter using `Cpp::ActivateInterpreter`, this information can be retrived using the map.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -126,15 +134,60 @@ struct InterpreterInfo { | |||
| }; | |||
|
|
|||
| // 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.
warning: variable 'sInterpreters' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
sInterpreters;
^| static llvm::ManagedStatic<std::deque<std::shared_ptr<InterpreterInfo>>> | ||
| sInterpreters; | ||
| static llvm::ManagedStatic< | ||
| std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>> |
There was a problem hiding this comment.
warning: no header providing "std::unordered_map" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:40:
- #if CLANG_VERSION_MAJOR >= 19
+ #include <unordered_map>
+ #if CLANG_VERSION_MAJOR >= 19| sInterpreters; | ||
| static llvm::ManagedStatic< | ||
| std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>> | ||
| sInterpreterASTMap; |
There was a problem hiding this comment.
warning: variable 'sInterpreterASTMap' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
sInterpreterASTMap;
^| static llvm::ManagedStatic< | ||
| std::unordered_map<clang::ASTContext*, std::weak_ptr<InterpreterInfo>>> | ||
| sInterpreterASTMap; | ||
| static std::mutex InterpreterStackLock; |
There was a problem hiding this comment.
warning: variable 'InterpreterStackLock' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]
static std::mutex InterpreterStackLock;
^| if (item.first->getAllocator().identifyObject(D)) | ||
| return *item.second.lock(); | ||
| } | ||
| llvm_unreachable( |
There was a problem hiding this comment.
warning: no header providing "llvm_unreachable" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:40:
- #if CLANG_VERSION_MAJOR >= 19
+ #include <llvm/Support/ErrorHandling.h>
+ #if CLANG_VERSION_MAJOR >= 19| @@ -684,13 +757,14 @@ | |||
| TCppScope_t GetNamed(const std::string& name, | |||
| TCppScope_t parent /*= nullptr*/) { | |||
| clang::DeclContext* Within = 0; | |||
| auto* D = (clang::Decl*)parent; | |||
There was a problem hiding this comment.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* D = (clang::Decl*)parent;
^| D = GetUnderlyingScope(D); | ||
| Within = llvm::dyn_cast<clang::DeclContext>(D); | ||
| } | ||
|
|
||
| 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.
warning: no header providing "Cpp::utils::Lookup::Named" is directly included [misc-include-cleaner]
lib/CppInterOp/CppInterOp.cpp:12:
+ #include "CppInterOpInterpreter.h"| @@ -734,7 +811,11 @@ | |||
| TCppScope_t GetBaseClass(TCppScope_t klass, TCppIndex_t ibase) { | |||
| auto* D = (Decl*)klass; | |||
| auto* CXXRD = llvm::dyn_cast_or_null<CXXRecordDecl>(D); | |||
| if (!CXXRD || CXXRD->getNumBases() <= ibase) | |||
| if (!CXXRD) | |||
| return 0; | |||
There was a problem hiding this comment.
warning: use nullptr [modernize-use-nullptr]
| return 0; | |
| return nullptr; |
| return interp.getCI()->getSema().LookupDefaultConstructor(CXXRD); | ||
| } | ||
|
|
||
| TCppFunction_t GetDefaultConstructor(TCppScope_t scope) { | ||
| return GetDefaultConstructor(getInterp(), scope); | ||
| auto* CXXRD = (clang::CXXRecordDecl*)scope; |
There was a problem hiding this comment.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* CXXRD = (clang::CXXRecordDecl*)scope;
^|
|
||
| auto* D = (clang::Decl*)func; |
There was a problem hiding this comment.
warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
auto* D = (clang::Decl*)func;
^
Probably no need to pursue if we have a superior approach. |
Same as #698
But using
ASTContext::getAllocator::identifyObject.This implementation is terribly slow.