Raphael is correct that the idea behind all of the "CompilerXXXX" classes is to
abstract us from the TypeSystem that is being used, so they should stay "void
*". I would rather not have each CompilerXXXX class make a union of pointers.
CompilerType looks like:
class CompilerType {
...
private:
lldb::opaque_compiler_type_t m_type = nullptr;
TypeSystem *m_type_system = nullptr;
};
If would rather not see:
class CompilerType {
...
private:
union {
clang::Type*clang;
swift::Type *swift;
};
TypeSystem *m_type_system = nullptr;
};
Because then every type systems that gets installed has to modify this file and
add forward declarations. It is very nice that if we don't compile in the Swift
support that we don't need to change CompilerType and anyone that comes along
(rust, go, etc) can have their own type systems that are completely abstracted
without any need to modify CompilerType.h, CompilerDecl.h, etc...
> On Jan 3, 2020, at 12:50 AM, Raphael “Teemperor” Isemann via lldb-commits
> <lldb-commits@lists.llvm.org> wrote:
>
> We could use clang::Decl* etc. in these classes but that would defeat the
> whole concept that they are supposed to implement. CompilerType,
> CompilerDecl, etc.. are all classes that represent their respective
> equivalent in a language plugin. For Clang that is indeed clang::Type* and
> clang::Decl*, but Swift is storing swift::Type* and swift::Decl* in these
> void* pointers (and passes a SwiftASTContext* instead of a ClangASTContext*
> as a m_type_system). So if we change that to clang::Decl* then the all other
> language plugins like Swift are effectively broken. It would also mean that
> generic LLDB code using these classes would have a hard dependency on Clang.
>
> And there shouldn’t be any place where we unconditionally cast this to
> clang::Decl* as this would directly crash swift-lldb. The m_type_system
> (which is either ClangASTContext or SwiftASTContext) can freely cast the
> void* to clang::Decl/swift::Decl as the checking happens through the virtual
> function call to the m_type_system interface (Swift types always have a
> SwiftASTContext as m_type_system, Clang types have a ClangASTContext as
> m_type_system).
>
> Having said that, I’m not saying that the void* pointers in these classes are
> the best solution we could have here (there is a reason why I keep removing
> all these void* pointer conversions). If you see a way to get rid of them,
> then be my guest.
>
>> On 3. Jan 2020, at 01:52, Shafik Yaghmour <syaghm...@apple.com> wrote:
>>
>> To further clarify all the member functions of CompilerDecl e.g.:
>>
>> GetMangledName(…)
>> GetDeclContext(…)
>>
>> End up passing m_opaque_decl as an argument to a member function of
>> m_type_system and in these member functions they unconditionally cast the
>> argument to Decl* so why not make m_opaque_decl a Decl*
>>
>>> On Jan 2, 2020, at 4:20 PM, Shafik Yaghmour via lldb-commits
>>> <lldb-commits@lists.llvm.org> wrote:
>>>
>>> Apologies, m_opaque_decl member of CompilerDecl and operator < in
>>> CompilerDecl.h
>>>
>>>> On Jan 2, 2020, at 3:54 PM, Raphael “Teemperor” Isemann
>>>> <teempe...@gmail.com> wrote:
>>>>
>>>> Not sure if I can follow. What variable should just be a Decl* and what
>>>> operator<?
>>>>
>>>> (Also yeah that reinterpret_cast could also be a static_cast)
>>>>
>>>>> On Jan 3, 2020, at 12:38 AM, Shafik Yaghmour <syaghm...@apple.com> wrote:
>>>>>
>>>>> I am not crazy about the reinterpret_cast although if we look inside
>>>>> CompilerDecl impl we can see that basically it is always assuming it is a
>>>>> Decl* and just C-style casts it as such. So why not just make it a Decl*
>>>>>
>>>>> Also operator<(…) is super sketchy doing a < on void*
>>>>>
>>>>>> On Dec 28, 2019, at 6:52 AM, Raphael Isemann via lldb-commits
>>>>>> <lldb-commits@lists.llvm.org> wrote:
>>>>>>
>>>>>>
>>>>>> Author: Raphael Isemann
>>>>>> Date: 2019-12-28T15:20:19+01:00
>>>>>> New Revision: 8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
>>>>>>
>>>>>> URL:
>>>>>> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15
>>>>>> DIFF:
>>>>>> https://github.com/llvm/llvm-project/commit/8612e92ed590e615f9f56e4fb86a1fdaf3a39e15.diff
>>>>>>
>>>>>> LOG: [lldb][NFC] Remove GetASTContext call in ClangDeclVendor
>>>>>>
>>>>>> Instead of returning NamedDecls and then calling GetASTContext
>>>>>> to find back the ClangASTContext we used can just implement the
>>>>>> FindDecl variant that returns CompilerDecls (and implement the
>>>>>> other function by throwing away the ClangASTContext part of the
>>>>>> compiler decl).
>>>>>>
>>>>>> Added:
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>>>>>> lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>>>>>> lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>>>>>> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
>>>>>> lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
>>>>>>
>>>>>> Removed:
>>>>>>
>>>>>>
>>>>>>
>>>>>> ################################################################################
>>>>>> diff --git
>>>>>> a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>>>>>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>>>>>> index c59722b7b4f8..0c5796650d45 100644
>>>>>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>>>>>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.cpp
>>>>>> @@ -15,16 +15,17 @@ using namespace lldb_private;
>>>>>>
>>>>>> uint32_t ClangDeclVendor::FindDecls(ConstString name, bool append,
>>>>>> uint32_t max_matches,
>>>>>> - std::vector<CompilerDecl> &decls) {
>>>>>> + std::vector<clang::NamedDecl *>
>>>>>> &decls) {
>>>>>> if (!append)
>>>>>> decls.clear();
>>>>>>
>>>>>> - std::vector<clang::NamedDecl *> named_decls;
>>>>>> - uint32_t ret = FindDecls(name, /*append*/ false, max_matches,
>>>>>> named_decls);
>>>>>> - for (auto *named_decl : named_decls) {
>>>>>> - decls.push_back(CompilerDecl(
>>>>>> - ClangASTContext::GetASTContext(&named_decl->getASTContext()),
>>>>>> - named_decl));
>>>>>> + std::vector<CompilerDecl> compiler_decls;
>>>>>> + uint32_t ret = FindDecls(name, /*append*/ false, max_matches,
>>>>>> compiler_decls);
>>>>>> + for (CompilerDecl compiler_decl : compiler_decls) {
>>>>>> + clang::Decl *d =
>>>>>> + reinterpret_cast<clang::Decl *>(compiler_decl.GetOpaqueDecl());
>>>>>> + clang::NamedDecl *nd = llvm::cast<clang::NamedDecl>(d);
>>>>>> + decls.push_back(nd);
>>>>>> }
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> diff --git
>>>>>> a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>>>>>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>>>>>> index 85a10400a201..0c888de08841 100644
>>>>>> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>>>>>> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangDeclVendor.h
>>>>>> @@ -21,12 +21,10 @@ class ClangDeclVendor : public DeclVendor {
>>>>>>
>>>>>> virtual ~ClangDeclVendor() {}
>>>>>>
>>>>>> - uint32_t FindDecls(ConstString name, bool append, uint32_t
>>>>>> max_matches,
>>>>>> - std::vector<CompilerDecl> &decls) override;
>>>>>> + using DeclVendor::FindDecls;
>>>>>>
>>>>>> - virtual uint32_t FindDecls(ConstString name, bool append,
>>>>>> - uint32_t max_matches,
>>>>>> - std::vector<clang::NamedDecl *> &decls) =
>>>>>> 0;
>>>>>> + uint32_t FindDecls(ConstString name, bool append, uint32_t
>>>>>> max_matches,
>>>>>> + std::vector<clang::NamedDecl *> &decls);
>>>>>>
>>>>>> static bool classof(const DeclVendor *vendor) {
>>>>>> return vendor->GetKind() >= eClangDeclVendor &&
>>>>>>
>>>>>> diff --git
>>>>>> a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>>>>>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>>>>>> index ff0905dda4ef..0696c669f2e2 100644
>>>>>> ---
>>>>>> a/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>>>>>> +++
>>>>>> b/lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
>>>>>> @@ -80,7 +80,7 @@ class ClangModulesDeclVendorImpl : public
>>>>>> ClangModulesDeclVendor {
>>>>>> Stream &error_stream) override;
>>>>>>
>>>>>> uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
>>>>>> - std::vector<clang::NamedDecl *> &decls) override;
>>>>>> + std::vector<CompilerDecl> &decls) override;
>>>>>>
>>>>>> void ForEachMacro(const ModuleVector &modules,
>>>>>> std::function<bool(const std::string &)> handler)
>>>>>> override;
>>>>>> @@ -356,7 +356,7 @@ bool
>>>>>> ClangModulesDeclVendorImpl::AddModulesForCompileUnit(
>>>>>> uint32_t
>>>>>> ClangModulesDeclVendorImpl::FindDecls(ConstString name, bool append,
>>>>>> uint32_t max_matches,
>>>>>> - std::vector<clang::NamedDecl *>
>>>>>> &decls) {
>>>>>> + std::vector<CompilerDecl> &decls)
>>>>>> {
>>>>>> if (!m_enabled) {
>>>>>> return 0;
>>>>>> }
>>>>>> @@ -382,7 +382,7 @@ ClangModulesDeclVendorImpl::FindDecls(ConstString
>>>>>> name, bool append,
>>>>>> if (num_matches >= max_matches)
>>>>>> return num_matches;
>>>>>>
>>>>>> - decls.push_back(named_decl);
>>>>>> + decls.push_back(CompilerDecl(m_ast_context.get(), named_decl));
>>>>>> ++num_matches;
>>>>>> }
>>>>>>
>>>>>>
>>>>>> diff --git
>>>>>> a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
>>>>>>
>>>>>> b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
>>>>>> index 54f8397e1dad..29930c303b07 100644
>>>>>> ---
>>>>>> a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
>>>>>> +++
>>>>>> b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
>>>>>> @@ -537,10 +537,9 @@ bool
>>>>>> AppleObjCDeclVendor::FinishDecl(clang::ObjCInterfaceDecl
>>>>>> *interface_decl) {
>>>>>> return true;
>>>>>> }
>>>>>>
>>>>>> -uint32_t
>>>>>> -AppleObjCDeclVendor::FindDecls(ConstString name, bool append,
>>>>>> - uint32_t max_matches,
>>>>>> - std::vector<clang::NamedDecl *> &decls) {
>>>>>> +uint32_t AppleObjCDeclVendor::FindDecls(ConstString name, bool append,
>>>>>> + uint32_t max_matches,
>>>>>> + std::vector<CompilerDecl>
>>>>>> &decls) {
>>>>>> static unsigned int invocation_id = 0;
>>>>>> unsigned int current_id = invocation_id++;
>>>>>>
>>>>>> @@ -587,7 +586,7 @@ AppleObjCDeclVendor::FindDecls(ConstString name,
>>>>>> bool append,
>>>>>> current_id, result_iface_type.getAsString(), isa_value);
>>>>>> }
>>>>>>
>>>>>> - decls.push_back(result_iface_decl);
>>>>>> + decls.push_back(CompilerDecl(&m_ast_ctx, result_iface_decl));
>>>>>> ret++;
>>>>>> break;
>>>>>> } else {
>>>>>> @@ -630,7 +629,7 @@ AppleObjCDeclVendor::FindDecls(ConstString name,
>>>>>> bool append,
>>>>>> new_iface_type.getAsString(), (uint64_t)isa);
>>>>>> }
>>>>>>
>>>>>> - decls.push_back(iface_decl);
>>>>>> + decls.push_back(CompilerDecl(&m_ast_ctx, iface_decl));
>>>>>> ret++;
>>>>>> break;
>>>>>> } while (false);
>>>>>>
>>>>>> diff --git
>>>>>> a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
>>>>>>
>>>>>> b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
>>>>>> index 311113a27735..f49ca3540c2c 100644
>>>>>> ---
>>>>>> a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
>>>>>> +++
>>>>>> b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.h
>>>>>> @@ -28,7 +28,7 @@ class AppleObjCDeclVendor : public ClangDeclVendor {
>>>>>> }
>>>>>>
>>>>>> uint32_t FindDecls(ConstString name, bool append, uint32_t max_matches,
>>>>>> - std::vector<clang::NamedDecl *> &decls) override;
>>>>>> + std::vector<CompilerDecl> &decls) override;
>>>>>>
>>>>>> friend class AppleObjCExternalASTSource;
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> lldb-commits mailing list
>>>>>> lldb-commits@lists.llvm.org
>>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>>>
>>>>
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits@lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
