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

2020-02-18 Thread Raphael Isemann via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf9568a95493a: [lldb][NFC] Make all CompilerDeclContext 
parameters references instead of… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74607

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBModule.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/AddressResolverName.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFile.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -395,7 +395,8 @@
   if (Context.empty())
 return CompilerDeclContext();
   VariableList List;
-  Symfile.FindGlobalVariables(ConstString(Context), nullptr, UINT32_MAX, List);
+  Symfile.FindGlobalVariables(ConstString(Context), CompilerDeclContext(),
+  UINT32_MAX, List);
   if (List.Empty())
 return make_string_error("Context search didn't find a match.");
   if (List.GetSize() > 1)
@@ -442,8 +443,8 @@
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
-CompilerDeclContext *ContextPtr =
-ContextOr->IsValid() ? &*ContextOr : nullptr;
+const CompilerDeclContext &ContextPtr =
+ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 List.Clear();
 Symfile.FindFunctions(ConstString(Name), ContextPtr, getFunctionNameFlags(),
@@ -498,8 +499,8 @@
   Expected ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
 return ContextOr.takeError();
-  CompilerDeclContext *ContextPtr =
-  ContextOr->IsValid() ? &*ContextOr : nullptr;
+  const CompilerDeclContext &ContextPtr =
+  ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
   CompilerDeclContext Result =
   Symfile.FindNamespace(ConstString(Name), ContextPtr);
@@ -516,8 +517,8 @@
   Expected ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
 return ContextOr.takeError();
-  CompilerDeclContext *ContextPtr =
-  ContextOr->IsValid() ? &*ContextOr : nullptr;
+  const CompilerDeclContext &ContextPtr =
+  ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
   LanguageSet languages;
   if (!Language.empty())
@@ -566,8 +567,8 @@
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
-CompilerDeclContext *ContextPtr =
-ContextOr->IsValid() ? &*ContextOr : nullptr;
+const CompilerDeclContext &ContextPtr =
+ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 Symfile.FindGlobalVariables(ConstString(Name), ContextPtr, UINT32_MAX, List);
   }
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -105,7 +105,7 @@
 }
 
 void SymbolFile::FindGlobalVariables(ConstString name,
- const CompilerDeclContext *parent_decl_ctx,
+ const CompilerDeclContext &parent_decl_ctx,
  uint32_t max_matches,
  VariableList &variables) {}
 
@@ -114,7 +114,7 @@
  VariableList &variables) {}
 
 void SymbolFile::FindFunctions(ConstString name,
-   const CompilerDeclContext *parent_decl_ctx,
+   const CompilerDeclConte

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

2020-02-17 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor updated this revision to Diff 244921.
teemperor added a comment.

- Readded some `parent_decl_context.IsValid()` checks to SymbolFilePDB


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

https://reviews.llvm.org/D74607

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBModule.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/AddressResolverName.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFile.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -395,7 +395,8 @@
   if (Context.empty())
 return CompilerDeclContext();
   VariableList List;
-  Symfile.FindGlobalVariables(ConstString(Context), nullptr, UINT32_MAX, List);
+  Symfile.FindGlobalVariables(ConstString(Context), CompilerDeclContext(),
+  UINT32_MAX, List);
   if (List.Empty())
 return make_string_error("Context search didn't find a match.");
   if (List.GetSize() > 1)
@@ -442,8 +443,8 @@
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
-CompilerDeclContext *ContextPtr =
-ContextOr->IsValid() ? &*ContextOr : nullptr;
+const CompilerDeclContext &ContextPtr =
+ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 List.Clear();
 Symfile.FindFunctions(ConstString(Name), ContextPtr, getFunctionNameFlags(),
@@ -498,8 +499,8 @@
   Expected ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
 return ContextOr.takeError();
-  CompilerDeclContext *ContextPtr =
-  ContextOr->IsValid() ? &*ContextOr : nullptr;
+  const CompilerDeclContext &ContextPtr =
+  ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
   CompilerDeclContext Result =
   Symfile.FindNamespace(ConstString(Name), ContextPtr);
@@ -516,8 +517,8 @@
   Expected ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
 return ContextOr.takeError();
-  CompilerDeclContext *ContextPtr =
-  ContextOr->IsValid() ? &*ContextOr : nullptr;
+  const CompilerDeclContext &ContextPtr =
+  ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
   LanguageSet languages;
   if (!Language.empty())
@@ -566,8 +567,8 @@
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
-CompilerDeclContext *ContextPtr =
-ContextOr->IsValid() ? &*ContextOr : nullptr;
+const CompilerDeclContext &ContextPtr =
+ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 Symfile.FindGlobalVariables(ConstString(Name), ContextPtr, UINT32_MAX, List);
   }
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -105,7 +105,7 @@
 }
 
 void SymbolFile::FindGlobalVariables(ConstString name,
- const CompilerDeclContext *parent_decl_ctx,
+ const CompilerDeclContext &parent_decl_ctx,
  uint32_t max_matches,
  VariableList &variables) {}
 
@@ -114,7 +114,7 @@
  VariableList &variables) {}
 
 void SymbolFile::FindFunctions(ConstString name,
-   const CompilerDeclContext *parent_decl_ctx,
+   const CompilerDeclContext &parent_decl_ctx,
lldb::FunctionNameType name_type_mask,

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

2020-02-14 Thread 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



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

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

2020-02-14 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik accepted this revision.
shafik added a comment.

This is a great change, it makes the code more consistent. LGTM besides the few 
comments I made.




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;

 `!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. 



Comment at: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1326
 
-if (parent_decl_ctx &&
-GetDeclContextContainingUID(id) != *parent_decl_ctx)
+if (GetDeclContextContainingUID(id) != parent_decl_ctx)
   continue;

Same comment as above.



Comment at: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1549
 
-if (parent_decl_ctx && GetDeclContextContainingUID(
-   result->getSymIndexId()) != *parent_decl_ctx)
+if (GetDeclContextContainingUID(result->getSymIndexId()) != 
parent_decl_ctx)
   continue;

same here.


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

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

2020-02-14 Thread Pavel Labath via Phabricator via lldb-commits 
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Yes, please.

Bonus points to anyone who can replace the default-constructed 
CompilerDeclContext with llvm::None.


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

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

2020-02-14 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor created this revision.
teemperor added reviewers: labath, mib.
Herald added subscribers: lldb-commits, JDevlieghere, abidh, arphaman.
Herald added a project: LLDB.

All of our lookup APIs either use `CompilerDeclContext &` or 
`CompilerDeclContext *` semi-randomly it seems.
This leads to us constantly converting between those two types (and doing 
nullptr checks when going from
pointer to reference). It also leads to the confusing situation where we have 
two possible ways to express
that we don't have a CompilerDeclContex: either a nullptr or an invalid 
CompilerDeclContext (aka a default
constructed CompilerDeclContext).

This moves all APIs to use references and gets rid of all the nullptr checks 
and conversions.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D74607

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/source/API/SBModule.cpp
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/AddressResolverName.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFile.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -395,7 +395,8 @@
   if (Context.empty())
 return CompilerDeclContext();
   VariableList List;
-  Symfile.FindGlobalVariables(ConstString(Context), nullptr, UINT32_MAX, List);
+  Symfile.FindGlobalVariables(ConstString(Context), CompilerDeclContext(),
+  UINT32_MAX, List);
   if (List.Empty())
 return make_string_error("Context search didn't find a match.");
   if (List.GetSize() > 1)
@@ -442,8 +443,8 @@
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
-CompilerDeclContext *ContextPtr =
-ContextOr->IsValid() ? &*ContextOr : nullptr;
+const CompilerDeclContext &ContextPtr =
+ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 List.Clear();
 Symfile.FindFunctions(ConstString(Name), ContextPtr, getFunctionNameFlags(),
@@ -498,8 +499,8 @@
   Expected ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
 return ContextOr.takeError();
-  CompilerDeclContext *ContextPtr =
-  ContextOr->IsValid() ? &*ContextOr : nullptr;
+  const CompilerDeclContext &ContextPtr =
+  ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
   CompilerDeclContext Result =
   Symfile.FindNamespace(ConstString(Name), ContextPtr);
@@ -516,8 +517,8 @@
   Expected ContextOr = getDeclContext(Symfile);
   if (!ContextOr)
 return ContextOr.takeError();
-  CompilerDeclContext *ContextPtr =
-  ContextOr->IsValid() ? &*ContextOr : nullptr;
+  const CompilerDeclContext &ContextPtr =
+  ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
   LanguageSet languages;
   if (!Language.empty())
@@ -566,8 +567,8 @@
 Expected ContextOr = getDeclContext(Symfile);
 if (!ContextOr)
   return ContextOr.takeError();
-CompilerDeclContext *ContextPtr =
-ContextOr->IsValid() ? &*ContextOr : nullptr;
+const CompilerDeclContext &ContextPtr =
+ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 Symfile.FindGlobalVariables(ConstString(Name), ContextPtr, UINT32_MAX, List);
   }
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -105,7 +105,7 @@
 }
 
 void SymbolFile::FindGlobalVariables(ConstString name,
- const CompilerDeclContext *parent_decl_ctx,
+