[Lldb-commits] [PATCH] D74607: [lldb][NFC] Make all CompilerDeclContext parameters references instead of pointers

[Raphael Isemann via Phabricator via lldb-commits]

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