[Lldb-commits] [PATCH] D66451: [ClangExpressionParser] Add ClangDeclVendor

[Alex Langford via Phabricator via lldb-commits]

xiaobai marked an inline comment as done.
xiaobai added inline comments.


================
Comment at: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:110
     do {
-      DeclVendor *modules_decl_vendor =
-          m_target->GetClangModulesDeclVendor();
+      auto *modules_decl_vendor = llvm::cast<ClangModulesDeclVendor>(
+          m_target->GetClangModulesDeclVendor());
----------------
shafik wrote:
> This part of the change was recently reverted, see: http://llvm.org/viewvc/llvm-project?view=revision&revision=372974
> 
> I am wondering if the change right above for `runtime_decl_vendor` is also problematic as well. Can you explain the rationale?
> 
> CC @teemperor 
> 
> 
This is actually a bug. I intended to cast it to a `ClangDeclVendor`. The instance above for `runtime_decl_vendor` shouldn't be problematic. The rationale behind this change was to remove the clang-specific code in the DeclVendor class by introducing a ClangDeclVendor class to sit in between DeclVendor and its clang-specific subclasses (ClangModulesDeclVendor and AppleObjCDeclVendor). That partial revert is probably fine honestly since it's using ClangModulesDeclVendor directly.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66451/new/

https://reviews.llvm.org/D66451