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


================
Comment at: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1133
 
-    if (parent_decl_ctx && GetDeclContextContainingUID(
-                               result->getSymIndexId()) != *parent_decl_ctx)
+    if (GetDeclContextContainingUID(result->getSymIndexId()) != 
parent_decl_ctx)
       continue;
----------------
shafik wrote:
>  `!parent_decl_ctx.IsValid() && 
> GetDeclContextContainingUID(result->getSymIndexId()) != parent_decl_ctx` 
> seems like a more accurate replacement  especially if 
> `GetDeclContextContainingUID` can also return an invalid result, I think. 
I think the negation at the start is a typo? With the negation it would mean 
that with a valid `parent_decl_ctx` the continue would never be evaluated. But 
the intend seems to be to only parse variables that are in the DeclContext. So 
with the negation, specifying any `parent_decl_ctx`would disable the filter and 
we would parse all variables in all DeclContexts?

But yeah, I think this makes sense. In theory 
`DeclContextMatchesThisSymbolFile` seems to early-exit on the error cases that 
would cause `GetDeclContextContainingUID` to return an invalid DeclContext and 
the function has an assert for an valid result at the end, but I think the 
`IPDBSession` error case could return an invalid DeclContext in this function. 
Let's just play safe and keep the check.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D74607



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

Reply via email to