[Lldb-commits] [PATCH] D61548: Fix use of 'is' operator for comparison

[Raul Tambre via Phabricator via lldb-commits]

tambre created this revision.
tambre added a reviewer: zturner.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The 'is' operator is not meant to be used for comparisons. It currently working 
is an implementation detail of CPython.
CPython 3.8 has added a SyntaxWarning for this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D61548

Files:
  lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py


Index: lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
===
--- lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
@@ -79,9 +79,9 @@
 "target variable returns wrong variable " + name)
 
 for name in global_variables_assert:
-if name is "C::a" and not test_c:
+if name == "C::a" and not test_c:
 continue
-if name is not "C::a" and test_c:
+if name != "C::a" and test_c:
 continue
 
 value = frame.EvaluateExpression(name)


Index: lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
===
--- lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
@@ -79,9 +79,9 @@
 "target variable returns wrong variable " + name)
 
 for name in global_variables_assert:
-if name is "C::a" and not test_c:
+if name == "C::a" and not test_c:
 continue
-if name is not "C::a" and test_c:
+if name != "C::a" and test_c:
 continue
 
 value = frame.EvaluateExpression(name)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Aleksei Sidorin via Phabricator via lldb-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

Hello Gabor!
This looks good to me, but let's wait for LLDB guys to take a look at the 
patch. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333



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

[Lldb-commits] [PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Aleksei Sidorin via Phabricator via lldb-commits]

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

👍




Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:277
   merger.RemoveSources(importer_source);
-  return ret;
+  if (ret_or_error)
+return *ret_or_error;

'true' body needs to be surrounded by braces as well. Same below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438



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

[Lldb-commits] [PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added inline comments.



Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp:1980
+  }
+  return *type_or_error;
 }

>>! In D61438#1490102, @jingham wrote:
> [...] include the contents of that error n the log message?
e.g:
```
if (auto type_owner = merger.ImporterForOrigin(from_context).Import(type)) {
  return *type_owner;
} else {
  Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS);
  LLDB_LOG_ERROR(log, type_owner.takeError(), "Couldn't import type: {0}")
  return QualType();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61438



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

[Lldb-commits] [PATCH] D61498: 01/06: Merge GetCompileUnit + GetCompileUnitContainingDIEOffset

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil updated this revision to Diff 198148.
jankratochvil added a comment.

In D61498#1489682 , @labath wrote:

> What you could do instead is have a single function which returns the CU if 
> it is in the range `cu.GetOffset()  <= input < cu.GetNextUnitOffset()`, then 
> the wrapper functions could just perform a more stricter check on the 
> returned cu (I.e. no templates or callbacks).


OK, I agree; originally I did not want to touch more code than what I needed 
for .debug_types.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D61498

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -74,6 +74,8 @@
   // accessors are called.
   void ParseCompileUnitHeadersIfNeeded();
 
+  uint32_t FindCompileUnitIndex(dw_offset_t offset);
+
   DISALLOW_COPY_AND_ASSIGN(DWARFDebugInfo);
 };
 
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -123,37 +123,30 @@
   return offset < cu_sp->GetOffset();
 }
 
+uint32_t DWARFDebugInfo::FindCompileUnitIndex(dw_offset_t offset) {
+  ParseCompileUnitHeadersIfNeeded();
+
+  // llvm::lower_bound is not used as for DIE offsets it would still return
+  // index +1 and GetOffset() returning index itself would be a special case.
+  auto pos = llvm::upper_bound(m_compile_units, offset,
+   OffsetLessThanCompileUnitOffset);
+  uint32_t idx = std::distance(m_compile_units.begin(), pos);
+  if (idx == 0)
+return DW_INVALID_OFFSET;
+  return idx - 1;
+}
+
 DWARFUnit *DWARFDebugInfo::GetCompileUnit(dw_offset_t cu_offset,
- uint32_t *idx_ptr) {
-  DWARFUnitSP cu_sp;
-  uint32_t cu_idx = DW_INVALID_INDEX;
-  if (cu_offset != DW_INVALID_OFFSET) {
-ParseCompileUnitHeadersIfNeeded();
-
-// Watch out for single compile unit executable as they are pretty common
-const size_t num_cus = m_compile_units.size();
-if (num_cus == 1) {
-  if (m_compile_units[0]->GetOffset() == cu_offset) {
-cu_sp = m_compile_units[0];
-cu_idx = 0;
-  }
-} else if (num_cus) {
-  CompileUnitColl::const_iterator end_pos = m_compile_units.end();
-  CompileUnitColl::const_iterator begin_pos = m_compile_units.begin();
-  CompileUnitColl::const_iterator pos = std::upper_bound(
-  begin_pos, end_pos, cu_offset, OffsetLessThanCompileUnitOffset);
-  if (pos != begin_pos) {
---pos;
-if ((*pos)->GetOffset() == cu_offset) {
-  cu_sp = *pos;
-  cu_idx = std::distance(begin_pos, pos);
-}
-  }
-}
+  uint32_t *idx_ptr) {
+  uint32_t idx = FindCompileUnitIndex(cu_offset);
+  DWARFUnit *result = GetCompileUnitAtIndex(idx);
+  if (result && result->GetOffset() != cu_offset) {
+result = nullptr;
+idx = DW_INVALID_INDEX;
   }
   if (idx_ptr)
-*idx_ptr = cu_idx;
-  return cu_sp.get();
+*idx_ptr = idx;
+  return result;
 }
 
 DWARFUnit *DWARFDebugInfo::GetCompileUnit(const DIERef &die_ref) {
@@ -165,28 +158,11 @@
 
 DWARFUnit *
 DWARFDebugInfo::GetCompileUnitContainingDIEOffset(dw_offset_t die_offset) {
-  ParseCompileUnitHeadersIfNeeded();
-
-  DWARFUnitSP cu_sp;
-
-  // Watch out for single compile unit executable as they are pretty common
-  const size_t num_cus = m_compile_units.size();
-  if (num_cus == 1) {
-if (m_compile_units[0]->ContainsDIEOffset(die_offset))
-  return m_compile_units[0].get();
-  } else if (num_cus) {
-CompileUnitColl::const_iterator end_pos = m_compile_units.end();
-CompileUnitColl::const_iterator begin_pos = m_compile_units.begin();
-CompileUnitColl::const_iterator pos = std::upper_bound(
-begin_pos, end_pos, die_offset, OffsetLessThanCompileUnitOffset);
-if (pos != begin_pos) {
-  --pos;
-  if ((*pos)->ContainsDIEOffset(die_offset))
-return (*pos).get();
-}
-  }
-
-  return nullptr;
+  uint32_t idx = FindCompileUnitIndex(die_offset);
+  DWARFUnit *result = GetCompileUnitAtIndex(idx);
+  if (result && !result->ContainsDIEOffset(die_offset))
+return nullptr;
+  return result;
 }
 
 DWARFDIE
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits