[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

[Zequan Wu via Phabricator via lldb-commits]

zequanwu abandoned this revision.
zequanwu added a comment.

D113930  is the correct fix for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113533

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

I'm pretty sure you're trying to solve the same problem as here: D85993 


In short: You're calling `CreateFunctionDeclaration` to create a function in a 
record, but you actually want to call `AddMethodToCXXRecordType` which allows 
passing and setting an access specifier (which is what the assert here checks). 
You can fix this in the PDB plugin by looking at the DeclContext and if 
`decl_ctx->isRecord()` -> `AddMethodToCXXRecordType` and otherwise 
`CreateFunctionDeclaration`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113533

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik added inline comments.



Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:95
-  // that Clang calls its internal Decl::AccessDeclContextSanity check.
-  decl->getAccess();
-#endif

No, we don't care about the return value but we care about the `assert` which 
means the AST node is not well formed. In this case IIRC we are verifying that 
a member has an access specifier that is not set to `AS_none`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113533

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D113533: [LLDB] Remove access check of decl in TypeSystemClang.cpp

[Zequan Wu via Phabricator via lldb-commits]

zequanwu created this revision.
zequanwu added reviewers: labath, shafik.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

As the deleted comment says, we don't care about the access specifier.
I also encountered a crash at here (called from 
https://github.com/llvm/llvm-project/blob/main/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L2141)
 when fixing a bug on SymbolFileNativePDB.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113533

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -89,11 +89,6 @@
 namespace {
 static void VerifyDecl(clang::Decl *decl) {
   assert(decl && "VerifyDecl called with nullptr?");
-#ifndef NDEBUG
-  // We don't care about the actual access value here but only want to trigger
-  // that Clang calls its internal Decl::AccessDeclContextSanity check.
-  decl->getAccess();
-#endif
 }
 
 static inline bool


Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -89,11 +89,6 @@
 namespace {
 static void VerifyDecl(clang::Decl *decl) {
   assert(decl && "VerifyDecl called with nullptr?");
-#ifndef NDEBUG
-  // We don't care about the actual access value here but only want to trigger
-  // that Clang calls its internal Decl::AccessDeclContextSanity check.
-  decl->getAccess();
-#endif
 }
 
 static inline bool
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits