[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Pavel Kosov via Phabricator via lldb-commits]

kpdev42 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2212
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-if (record_decl)
+if (record_decl) {
+  bool is_empty = true;

Michael137 wrote:
> Generally I'm not sure if attaching a `clang::NoUniqueAddressAttr` to every 
> empty field is the right approach. That goes slightly against our attempts to 
> construct an AST that's faithful to the source to avoid unpredictable 
> behaviour (which isn't always possible but for the most part we try). This 
> approach was considered in https://reviews.llvm.org/D101237 but concern was 
> raised about it affecting ABI, etc., leading to subtle issues down the line.
> 
> Based on the the discussion in https://reviews.llvm.org/D101237 it seemed to 
> me like the only two viable solutions are:
> 1. Add a `DW_AT_byte_size` of `0` to the empty field
> 2. Add a `DW_AT_no_unique_address`
> 
> AFAICT Jan tried to implement (1) but never seemed to be able to fully add 
> support for this in the ASTImporter/LLDB. Another issue I see with this is 
> that sometimes the byte-size of said field is not `0`, depending on the 
> context in which the structure is used.
> 
> I'm still leaning towards proposing a `DW_AT_no_unique_address`. Which is 
> pretty easy to implement and also reason about from LLDB's perspective. 
> @dblaikie @aprantl does that sound reasonable to you?
I think that lldb jitter relies on value of `DW_AT_member` location when/if 
empty structure address is taken, so assigning no_unique_address attribute 
shouldn't, in my opinion, affect anything. Also, as I understand, AST obtained 
from DWARF will not (and cannot) be identical to the source (e.g. because of 
optimizations)



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2219
+if (is_empty_field)
+  field->addAttr(clang::NoUniqueAddressAttr::Create(
+  m_ast.getASTContext(), clang::SourceRange()));

Michael137 wrote:
> Typically the call to `record_decl->fields()` below would worry me, because 
> if the decl `!hasLoadedFieldsFromExternalStorage()` then we'd start another 
> `ASTImport` process, which could lead to some unpredictable behaviour if we 
> are already in the middle of an import. But since 
> `CompleteTagDeclarationDefinition` sets 
> `setHasLoadedFieldsFromExternalStorage(true)` I *think* we'd be ok. Might 
> warrant a comment.
We can probably use keys of `DenseMap` in `layout_info.base_offsets` to stay 
safe, can't we?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2231
+if (is_empty)
+  record_decl->markEmpty();
+  }

Michael137 wrote:
> Why do we need to mark the parents empty here again? Wouldn't they have been 
> marked in `ParseStructureLikeDIE`?
`ParseStructureLikeDIE` marks only trivially empty records (with no children or 
with only children being template parameters). All non-trivially empty structs 
(with only children being other empty structs) are marked here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D147642: [lldb][ObjectFileELF] Support AArch32 in ApplyRelocations

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

sgraenitz added a comment.

Double-checked and found source locations on Raspberry Pi 3b with this patch 
applied 
https://github.com/llvm/llvm-project/issues/61948#issuecomment-1508305542
Is there more feedback on this? It would be nice to land it soon, i.e. before 
EuroLLVM =)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147642

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

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513531.
Michael137 added a comment.

- Only search for root namespace if we're doing a qualified lookup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  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/SymbolFileOnDemand.cpp
  lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,18 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +124,7 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+return Foo::myfunc(12) + bb.x() + ab.y();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,6 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
Index: lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
===
--- lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -46,7 +46,7 @@
 
 self.expect("expr A::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A'"])
 self.expect("expr A::B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
-self.expect("expr B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
+self.expect("expr B::other_var", error=True, substrs=["use of undeclared identifier 'B'"])
 
 # 'frame variable' can correctly distinguish between A::B::global_var and A::global_var
 gvars = self.target().FindGlobalVariables("A::global_var", 10)
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::Compiler

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513539.
Michael137 added a comment.

- Move doxygen comment
- Add source comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  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/SymbolFileOnDemand.cpp
  lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,30 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +136,9 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+void *bbp = &bb;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,10 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="int", result_value="-3")
+self.expect_expr("((::B::Foo*)&bb).x()", result_type="int", result_value="42")
Index: lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
===
--- lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -46,7 +46,7 @@
 
 self.expect("expr A::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A'"])
 self.expect("expr A::B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
-self.expect("expr B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
+self.expect("expr B::other_var", error=True, substrs=["use of undeclared identifier 'B'"])
 
 # 'frame variable' can correctly distinguish between A::B::global_var and A::global_var
 gvars = self.target().FindGlobalVariables("A::global_var", 10)
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return 

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513542.
Michael137 added a comment.

- Make condition more readable
- Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  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/SymbolFileOnDemand.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,30 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +136,9 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+void *bbp = &bb;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,10 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="int", result_value="-3")
+self.expect_expr("((::B::Foo*)&bb).x()", result_type="int", result_value="42")
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private::CompilerDeclContext &parent_decl_ctx,
+bool only_root_namespaces) override;
 
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
Index: lld

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513547.
Michael137 added a comment.

- Fix test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  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/SymbolFileOnDemand.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,31 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+Bar bar;
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +137,8 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,11 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+# FIXME: C++ unqualified namespace lookups currently not supported when instantiating types.
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="bool", result_value="false")
+self.expect_expr("((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42")
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private::CompilerDeclContext &parent_decl_ctx,
+bool only_root_namespaces) override;
 
   llvm::StringRef G

[Lldb-commits] [lldb] 09ba7b6 - [lldb] Add a sleep to TestObjectFileJSON

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-04-14T14:37:32+02:00
New Revision: 09ba7b605327812cfcec4f3c01d7fc232dee651d

URL: 
https://github.com/llvm/llvm-project/commit/09ba7b605327812cfcec4f3c01d7fc232dee651d
DIFF: 
https://github.com/llvm/llvm-project/commit/09ba7b605327812cfcec4f3c01d7fc232dee651d.diff

LOG: [lldb] Add a sleep to TestObjectFileJSON

The test fails when the two generated files have the same timestamp
(lldb uses second granularity).

Added: 


Modified: 
lldb/test/API/macosx/symbols/TestObjectFileJSON.py

Removed: 




diff  --git a/lldb/test/API/macosx/symbols/TestObjectFileJSON.py 
b/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
index 04f91ec62f88b..67d9f8e3c5d02 100644
--- a/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
+++ b/lldb/test/API/macosx/symbols/TestObjectFileJSON.py
@@ -7,6 +7,7 @@
 import uuid
 import os
 import shutil
+import time
 
 
 class TestObjectFileJSON(TestBase):
@@ -80,6 +81,9 @@ def test_module(self):
 }
 ],
 }
+
+# Sleep to ensure the new file has a 
diff erent timestamp
+time.sleep(2)
 self.emitJSON(data, json_object_file)
 
 module = target.AddModule(self.toModuleSpec(json_object_file))



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

[Lldb-commits] [PATCH] D148062: [lldb] Make ObjectFileJSON loadable as a module

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/test/API/macosx/symbols/TestObjectFileJSON.py:82
+}
+self.emitJSON(data, json_object_file)
+

The test was (95%!) flaky because the two generated files had the same 
timestamp (and so lldb considered them identical and did not reload). I fixed 
that by adding a sleep, but your intention was not to test file reloading then 
it would be better to just use different filenames.

(There could also be a bug somewhere, because I was under the impression that 
we should treat files with different UUIDs as distinct even if they happen to 
have the same timestamp.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148062

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

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via Phabricator via lldb-commits]

Michael137 marked an inline comment as not done.
Michael137 added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:551
   case lldb::eLanguageTypeC_plus_plus_14:
 lang_opts.CPlusPlus11 = true;
 m_compiler->getHeaderSearchOpts().UseLibcxx = true;

Michael137 wrote:
> aprantl wrote:
> > Why does this not set C++14? Because it's effectively equivalent?
> I think it's because we haven't done the work to make c++14 in the expression 
> evaluator work, but the default clang language is c++14. So we downgrade it 
> silently to c++11 here.
> 
> We should really fix that, but I think this broke some tests in the past when 
> we tried enabling it (see `8cb75745412e4bc9592d2409cc6cfa4a2940d9e7`, 
> https://reviews.llvm.org/D80308)
But looking at the specific test failures from back then there doesn't seem 
like a good reason for why we can't re-enable that patch now. And also add a 
C++17 version.

Let me try that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

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

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:548
+[[fallthrough]];
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:

aprantl wrote:
> Why no case C++17?
Fair point, can add one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

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

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added a comment.

> since ConstStrings live forever

So long as this is true, I don't see any pitfalls either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148175

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

[Lldb-commits] [lldb] ce7a54a - [lldb] Add `operator StringRef` to ConstString

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2023-04-14T09:08:19-06:00
New Revision: ce7a54a27c0960e057a6eaf968d6967f75590fb1

URL: 
https://github.com/llvm/llvm-project/commit/ce7a54a27c0960e057a6eaf968d6967f75590fb1
DIFF: 
https://github.com/llvm/llvm-project/commit/ce7a54a27c0960e057a6eaf968d6967f75590fb1.diff

LOG: [lldb] Add `operator StringRef` to ConstString

Add a `StringRef` conversion function to `ConstString`.

This will make using llvm, and other non-ConstString, APIs more convenient.

For demonstration, this updates Module.cpp.

Differential Revision: https://reviews.llvm.org/D148175

Added: 


Modified: 
lldb/include/lldb/Utility/ConstString.h
lldb/source/Core/Module.cpp

Removed: 




diff  --git a/lldb/include/lldb/Utility/ConstString.h 
b/lldb/include/lldb/Utility/ConstString.h
index fe23303e14da2..a4b959b14f15a 100644
--- a/lldb/include/lldb/Utility/ConstString.h
+++ b/lldb/include/lldb/Utility/ConstString.h
@@ -180,6 +180,9 @@ class ConstString {
 
   bool operator<(ConstString rhs) const;
 
+  // Implicitly convert \class ConstString instances to \class StringRef.
+  operator llvm::StringRef() const { return GetStringRef(); }
+
   /// Get the string value as a C string.
   ///
   /// Get the value of the contained string as a NULL terminated C string

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 8f9defabd76f7..17d8043852ab7 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -748,8 +748,7 @@ bool Module::LookupInfo::NameMatchesLookupInfo(
   // relatively inexpensive since no demangling is actually occuring. See
   // Mangled::SetValue for more context.
   const bool function_name_may_be_mangled =
-  Mangled::GetManglingScheme(function_name.GetStringRef()) !=
-  Mangled::eManglingSchemeNone;
+  Mangled::GetManglingScheme(function_name) != 
Mangled::eManglingSchemeNone;
   ConstString demangled_function_name = function_name;
   if (function_name_may_be_mangled) {
 Mangled mangled_function_name(function_name);
@@ -760,11 +759,10 @@ bool Module::LookupInfo::NameMatchesLookupInfo(
   // Otherwise just check that the demangled function name contains the
   // demangled user-provided name.
   if (Language *language = Language::FindPlugin(language_type))
-return language->DemangledNameContainsPath(m_name.GetStringRef(),
-   demangled_function_name);
+return language->DemangledNameContainsPath(m_name, 
demangled_function_name);
 
-  llvm::StringRef function_name_ref = demangled_function_name.GetStringRef();
-  return function_name_ref.contains(m_name.GetStringRef());
+  llvm::StringRef function_name_ref = demangled_function_name;
+  return function_name_ref.contains(m_name);
 }
 
 void Module::LookupInfo::Prune(SymbolContextList &sc_list,
@@ -803,7 +801,7 @@ void Module::LookupInfo::Prune(SymbolContextList &sc_list,
 CPlusPlusLanguage::MethodName cpp_method(full_name);
 if (cpp_method.IsValid()) {
   if (cpp_method.GetContext().empty()) {
-if (cpp_method.GetBasename().compare(m_name.GetStringRef()) != 0) {
+if (cpp_method.GetBasename().compare(m_name) != 0) {
   sc_list.RemoveContextAtIndex(i);
   continue;
 }
@@ -1026,8 +1024,8 @@ void Module::FindTypes(
   FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
  searched_symbol_files, typesmap);
   if (exact_match) {
-typesmap.RemoveMismatchedTypes(type_scope, name.GetStringRef(),
-   type_class, exact_match);
+typesmap.RemoveMismatchedTypes(type_scope, name, type_class,
+   exact_match);
   }
 }
   }
@@ -1132,7 +1130,7 @@ void Module::ReportWarningOptimization(
 return;
 
   StreamString ss;
-  ss << file_name.GetStringRef()
+  ss << file_name
  << " was compiled with optimization - stepping may behave "
 "oddly; variables may not be available.";
   Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
@@ -1668,7 +1666,7 @@ uint32_t Module::Hash() {
   llvm::raw_string_ostream id_strm(identifier);
   id_strm << m_arch.GetTriple().str() << '-' << m_file.GetPath();
   if (m_object_name)
-id_strm << '(' << m_object_name.GetStringRef() << ')';
+id_strm << '(' << m_object_name << ')';
   if (m_object_offset > 0)
 id_strm << m_object_offset;
   const auto mtime = llvm::sys::toTimeT(m_object_mod_time);
@@ -1682,7 +1680,7 @@ std::string Module::GetCacheKey() {
   llvm::raw_string_ostream strm(key);
   strm << m_arch.GetTriple().str() << '-' << m_file.GetFilename();
   if (m_object_name)
-strm << '(' << m_object_name.GetStringRef() << ')';
+strm << '(' << m_object_name << ')';
   strm << '-' << llvm::format_hex(Hash(), 10);
   return strm.str();
 }



___

[Lldb-commits] [PATCH] D148175: [lldb] Add `operator StringRef` to ConstString

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce7a54a27c09: [lldb] Add `operator StringRef` to ConstString 
(authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148175

Files:
  lldb/include/lldb/Utility/ConstString.h
  lldb/source/Core/Module.cpp


Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -748,8 +748,7 @@
   // relatively inexpensive since no demangling is actually occuring. See
   // Mangled::SetValue for more context.
   const bool function_name_may_be_mangled =
-  Mangled::GetManglingScheme(function_name.GetStringRef()) !=
-  Mangled::eManglingSchemeNone;
+  Mangled::GetManglingScheme(function_name) != 
Mangled::eManglingSchemeNone;
   ConstString demangled_function_name = function_name;
   if (function_name_may_be_mangled) {
 Mangled mangled_function_name(function_name);
@@ -760,11 +759,10 @@
   // Otherwise just check that the demangled function name contains the
   // demangled user-provided name.
   if (Language *language = Language::FindPlugin(language_type))
-return language->DemangledNameContainsPath(m_name.GetStringRef(),
-   demangled_function_name);
+return language->DemangledNameContainsPath(m_name, 
demangled_function_name);
 
-  llvm::StringRef function_name_ref = demangled_function_name.GetStringRef();
-  return function_name_ref.contains(m_name.GetStringRef());
+  llvm::StringRef function_name_ref = demangled_function_name;
+  return function_name_ref.contains(m_name);
 }
 
 void Module::LookupInfo::Prune(SymbolContextList &sc_list,
@@ -803,7 +801,7 @@
 CPlusPlusLanguage::MethodName cpp_method(full_name);
 if (cpp_method.IsValid()) {
   if (cpp_method.GetContext().empty()) {
-if (cpp_method.GetBasename().compare(m_name.GetStringRef()) != 0) {
+if (cpp_method.GetBasename().compare(m_name) != 0) {
   sc_list.RemoveContextAtIndex(i);
   continue;
 }
@@ -1026,8 +1024,8 @@
   FindTypes_Impl(name, CompilerDeclContext(), UINT_MAX,
  searched_symbol_files, typesmap);
   if (exact_match) {
-typesmap.RemoveMismatchedTypes(type_scope, name.GetStringRef(),
-   type_class, exact_match);
+typesmap.RemoveMismatchedTypes(type_scope, name, type_class,
+   exact_match);
   }
 }
   }
@@ -1132,7 +1130,7 @@
 return;
 
   StreamString ss;
-  ss << file_name.GetStringRef()
+  ss << file_name
  << " was compiled with optimization - stepping may behave "
 "oddly; variables may not be available.";
   Debugger::ReportWarning(std::string(ss.GetString()), debugger_id,
@@ -1668,7 +1666,7 @@
   llvm::raw_string_ostream id_strm(identifier);
   id_strm << m_arch.GetTriple().str() << '-' << m_file.GetPath();
   if (m_object_name)
-id_strm << '(' << m_object_name.GetStringRef() << ')';
+id_strm << '(' << m_object_name << ')';
   if (m_object_offset > 0)
 id_strm << m_object_offset;
   const auto mtime = llvm::sys::toTimeT(m_object_mod_time);
@@ -1682,7 +1680,7 @@
   llvm::raw_string_ostream strm(key);
   strm << m_arch.GetTriple().str() << '-' << m_file.GetFilename();
   if (m_object_name)
-strm << '(' << m_object_name.GetStringRef() << ')';
+strm << '(' << m_object_name << ')';
   strm << '-' << llvm::format_hex(Hash(), 10);
   return strm.str();
 }
Index: lldb/include/lldb/Utility/ConstString.h
===
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -180,6 +180,9 @@
 
   bool operator<(ConstString rhs) const;
 
+  // Implicitly convert \class ConstString instances to \class StringRef.
+  operator llvm::StringRef() const { return GetStringRef(); }
+
   /// Get the string value as a C string.
   ///
   /// Get the value of the contained string as a NULL terminated C string


Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -748,8 +748,7 @@
   // relatively inexpensive since no demangling is actually occuring. See
   // Mangled::SetValue for more context.
   const bool function_name_may_be_mangled =
-  Mangled::GetManglingScheme(function_name.GetStringRef()) !=
-  Mangled::eManglingSchemeNone;
+  Mangled::GetManglingScheme(function_name) != Mangled::eManglingSchemeNone;
   ConstString demangled_function_name = function_name;
   if (function_name_may_be_mangled) {
 Mangled mangled_function_name(function_name);
@@ -760,11 +759,10 @@
   // Otherwise just check that the demangled function name contains the
 

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd updated this revision to Diff 513614.
compnerd added a comment.

Address feedback


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

https://reviews.llvm.org/D147669

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -863,10 +863,14 @@
   for (const auto &entry : m_binary->export_directories()) {
 llvm::StringRef sym_name;
 if (auto err = entry.getSymbolName(sym_name)) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromExportTable - failed to get export 
"
-   "table entry name: {0}",
-   llvm::fmt_consume(std::move(err)));
+  if (log)
+log->Format(
+__FILE__, __func__,
+"ObjectFilePECOFF::AppendFromExportTable - failed to get export "
+"table entry name: {0}",
+llvm::fmt_consume(std::move(err)));
+  else
+llvm::consumeError(std::move(err));
   continue;
 }
 Symbol symbol;
@@ -884,10 +888,13 @@
   // it in symtab and make a note using the symbol name.
   llvm::StringRef forwarder_name;
   if (auto err = entry.getForwardTo(forwarder_name)) {
-LLDB_LOG(log,
- "ObjectFilePECOFF::AppendFromExportTable - failed to get "
- "forwarder name of forwarder export '{0}': {1}",
- sym_name, llvm::fmt_consume(std::move(err)));
+if (log)
+  log->Format(__FILE__, __func__,
+  "ObjectFilePECOFF::AppendFromExportTable - failed to get 
"
+  "forwarder name of forwarder export '{0}': {1}",
+  sym_name, llvm::fmt_consume(std::move(err)));
+else
+  llvm::consumeError(std::move(err));
 continue;
   }
   llvm::SmallString<256> new_name = 
{symbol.GetDisplayName().GetStringRef(),
@@ -899,10 +906,13 @@
 
 uint32_t function_rva;
 if (auto err = entry.getExportRVA(function_rva)) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromExportTable - failed to get "
-   "address of export entry '{0}': {1}",
-   sym_name, llvm::fmt_consume(std::move(err)));
+  if (log)
+log->Format(__FILE__, __func__,
+"ObjectFilePECOFF::AppendFromExportTable - failed to get "
+"address of export entry '{0}': {1}",
+sym_name, llvm::fmt_consume(std::move(err)));
+  else
+llvm::consumeError(std::move(err));
   continue;
 }
 // Skip the symbol if it doesn't look valid.


Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -863,10 +863,14 @@
   for (const auto &entry : m_binary->export_directories()) {
 llvm::StringRef sym_name;
 if (auto err = entry.getSymbolName(sym_name)) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromExportTable - failed to get export "
-   "table entry name: {0}",
-   llvm::fmt_consume(std::move(err)));
+  if (log)
+log->Format(
+__FILE__, __func__,
+"ObjectFilePECOFF::AppendFromExportTable - failed to get export "
+"table entry name: {0}",
+llvm::fmt_consume(std::move(err)));
+  else
+llvm::consumeError(std::move(err));
   continue;
 }
 Symbol symbol;
@@ -884,10 +888,13 @@
   // it in symtab and make a note using the symbol name.
   llvm::StringRef forwarder_name;
   if (auto err = entry.getForwardTo(forwarder_name)) {
-LLDB_LOG(log,
- "ObjectFilePECOFF::AppendFromExportTable - failed to get "
- "forwarder name of forwarder export '{0}': {1}",
- sym_name, llvm::fmt_consume(std::move(err)));
+if (log)
+  log->Format(__FILE__, __func__,
+  "ObjectFilePECOFF::AppendFromExportTable - failed to get "
+  "forwarder name of forwarder export '{0}': {1}",
+  sym_name, llvm::fmt_consume(std::move(err)));
+else
+  llvm::consumeError(std::move(err));
 continue;
   }
   llvm::SmallString<256> new_name = {symbol.GetDisplayName().GetStringRef(),
@@ -899,10 +906,13 @@
 
 uint32_t function_rva;
 if (auto err = entry.getExportRVA(function_rva)) {
-  LLDB_LOG(log,
-   "ObjectFilePECOFF::AppendFromExportTable - failed to get "
-   "address of export entry '{0}': {1}",
-   sym_name, llvm::fmt_consume(std::move(err)));
+  if (log)
+log->Form

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

@sgraenitz done :)


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

https://reviews.llvm.org/D147669

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

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 513621.
Michael137 added a comment.

- Add FIXME


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Target/Language.cpp
  lldb/test/API/lang/cpp/standards/cpp20/Makefile
  lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
  lldb/test/API/lang/cpp/standards/cpp20/main.cpp
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -205,7 +205,11 @@
 
   {"auto Foo[abi:abc]::operator<<>(int) &", "auto",
"Foo[abi:abc]", "operator<<>", "(int)", "&",
-   "Foo[abi:abc]::operator<<>"}};
+   "Foo[abi:abc]::operator<<>"},
+
+  {"auto A::operator<=>[abi:tag]()", "auto", "A",
+   "operator<=>[abi:tag]", "()", "",
+   "A::operator<=>[abi:tag]"}};
 
   for (const auto &test : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
@@ -227,7 +231,6 @@
   std::string test_cases[] = {
   "int Foo::operator[]<[10>()",
   "Foo::operator bool[10]()",
-  "auto A::operator<=>[abi:tag]()",
   "auto A::operator<<<(int)",
   "auto A::operator>>>(int)",
   "auto A::operator<<(int)",
@@ -356,10 +359,9 @@
   EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
   "f>", context, basename));
 
-  // We expect these cases to fail until we turn on C++2a
-  EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+  EXPECT_TRUE(CPlusPlusLanguage::ExtractContextAndIdentifier(
   "A::operator<=>", context, basename));
-  EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+  EXPECT_TRUE(CPlusPlusLanguage::ExtractContextAndIdentifier(
   "operator<=>", context, basename));
 }
 
Index: lldb/test/API/lang/cpp/standards/cpp20/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp20/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+struct Foo {
+  friend auto operator<=>(Foo const &, Foo const &) { return true; }
+};
+
+int main() { return Foo{} <=> Foo{}; }
Index: lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
@@ -0,0 +1,16 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCPP20Standard(TestBase):
+def test_cpp20(self):
+"""
+Tests that we can evaluate an expression in C++20 mode
+"""
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Foo{}", lldb.SBFileSpec("main.cpp"))
+
+self.expect("expr -l c++11 -- Foo{} <=> Foo{}", error=True, substrs=["'<=>' is a single token in C++20; add a space to avoid a change in behavior"])
+
+self.expect("expr -l c++20 -- Foo{} <=> Foo{}", substrs=["(bool) $0 = true"])
Index: lldb/test/API/lang/cpp/standards/cpp20/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/standards/cpp20/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++20
+
+include Makefile.rules
Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -267,6 +267,8 @@
   case eLanguageTypeC_plus_plus_03:
   case eLanguageTypeC_plus_plus_11:
   case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeC_plus_plus_17:
+  case eLanguageTypeC_plus_plus_20:
   case eLanguageTypeObjC_plus_plus:
 return true;
   default:
@@ -306,6 +308,8 @@
   case eLanguageTypeC_plus_plus_03:
   case eLanguageTypeC_plus_plus_11:
   case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeC_plus_plus_17:
+  case eLanguageTypeC_plus_plus_20:
   case eLanguageTypeObjC_plus_plus:
   case eLanguageTypeObjC:
 return true;
@@ -329,6 +333,8 @@
   case eLanguageTypeC_plus_plus_03:
   case eLanguageTypeC_plus_plus_11:
   case eLanguageTypeC_plus_plus_14:
+  case eLanguageTypeC_plus_plus_17:
+  case eLanguageTypeC_plus_plus_20:
 return eLanguageTypeC_plus_plus;
   case eLanguageTypeC:
   case eLanguageTypeC89:
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/Ty

[Lldb-commits] [lldb] 0301a49 - [lldb][Language] Add more language types

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-04-14T17:10:17+01:00
New Revision: 0301a492f43efc3a2c1fa1bddd16e46aac8ab77e

URL: 
https://github.com/llvm/llvm-project/commit/0301a492f43efc3a2c1fa1bddd16e46aac8ab77e
DIFF: 
https://github.com/llvm/llvm-project/commit/0301a492f43efc3a2c1fa1bddd16e46aac8ab77e.diff

LOG: [lldb][Language] Add more language types

Adds more languages to the `language_names` list in
preparation for adding support for C++20 expression
evaluation.

The language constants were taken from the DWARFv5
constants defined in LLVM's `Dwarf.def`. Two vendor
constants overlap with the DWARFv5 constants so bump
their values. Their actual value is not important,
whereas keeping the enum values consecutive is (since
they are used for array lookups).

Differential Revision: https://reviews.llvm.org/D143061

Added: 


Modified: 
lldb/include/lldb/lldb-enumerations.h
lldb/source/Target/Language.cpp

Removed: 




diff  --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 0781c622faaf..af65684e8889 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -479,12 +479,24 @@ enum LanguageType {
   eLanguageTypeC_plus_plus_14 = 0x0021, ///< ISO C++:2014.
   eLanguageTypeFortran03 = 0x0022,  ///< ISO Fortran 2003.
   eLanguageTypeFortran08 = 0x0023,  ///< ISO Fortran 2008.
+  eLanguageTypeRenderScript = 0x0024,
+  eLanguageTypeBLISS = 0x0025,
+  eLanguageTypeKotlin = 0x0026,
+  eLanguageTypeZig = 0x0027,
+  eLanguageTypeCrystal = 0x0028,
+  eLanguageTypeC_plus_plus_17 = 0x002a, ///< ISO C++:2017.
+  eLanguageTypeC_plus_plus_20 = 0x002b, ///< ISO C++:2020.
+  eLanguageTypeC17 = 0x002c,
+  eLanguageTypeFortran18 = 0x002d,
+  eLanguageTypeAda2005 = 0x002e,
+  eLanguageTypeAda2012 = 0x002f,
+
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler = 0x0024,   ///< Mips_Assembler.
+  eLanguageTypeMipsAssembler,   ///< Mips_Assembler.
   eNumLanguageTypes
 };
 

diff  --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index 61fcbb75bd78..9f3899003f0d 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -194,6 +194,21 @@ struct language_name_pair language_names[] = {
 {"c++14", eLanguageTypeC_plus_plus_14},
 {"fortran03", eLanguageTypeFortran03},
 {"fortran08", eLanguageTypeFortran08},
+{"renderscript", eLanguageTypeRenderScript},
+{"bliss", eLanguageTypeBLISS},
+{"kotlin", eLanguageTypeKotlin},
+{"zig", eLanguageTypeZig},
+{"crystal", eLanguageTypeCrystal},
+{"",
+ static_cast(
+ 0x0029)}, // Not yet taken by any language in the DWARF spec
+   // and thus has no entry in LanguageType
+{"c++17", eLanguageTypeC_plus_plus_17},
+{"c++20", eLanguageTypeC_plus_plus_20},
+{"c17", eLanguageTypeC17},
+{"fortran18", eLanguageTypeFortran18},
+{"ada2005", eLanguageTypeAda2005},
+{"ada2012", eLanguageTypeAda2012},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
 // Now synonyms, in arbitrary order



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

[Lldb-commits] [lldb] 89cd0e8 - [lldb] Allow evaluating expressions in C++20 mode

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-04-14T17:10:18+01:00
New Revision: 89cd0e8c267f57004a734c94ff15c6bd0facf646

URL: 
https://github.com/llvm/llvm-project/commit/89cd0e8c267f57004a734c94ff15c6bd0facf646
DIFF: 
https://github.com/llvm/llvm-project/commit/89cd0e8c267f57004a734c94ff15c6bd0facf646.diff

LOG: [lldb] Allow evaluating expressions in C++20 mode

This patch allows users to evaluate expressions using
`expr -l c++20`. Currently DWARF keeps the CU's at
`DW_AT_language` at `DW_LANG_C_plus_plus_14` even
when compiling with `-std=c++20`. So even in "C++20
programs" expression evaluation will by default be
performed in `C++11` mode for now.

Enabling `C++14` has been previously attempted at
https://reviews.llvm.org/D80308

There are some remaining issues around evaluating C++20
expressions. Mainly, lack of support for C++20 AST nodes in
`clang::ASTImporter`. But these can be addressed in follow-up
patches.

Added: 
lldb/test/API/lang/cpp/standards/cpp20/Makefile
lldb/test/API/lang/cpp/standards/cpp20/TestCPP20Standard.py
lldb/test/API/lang/cpp/standards/cpp20/main.cpp

Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Target/Language.cpp
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index 9852bbc62aa4..79666ffadb08 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -509,6 +509,16 @@ ClangExpressionParser::ClangExpressionParser(
 // be re-evaluated in the future.
 lang_opts.CPlusPlus11 = true;
 break;
+  case lldb::eLanguageTypeC_plus_plus_20:
+lang_opts.CPlusPlus20 = true;
+LLVM_FALLTHROUGH;
+  case lldb::eLanguageTypeC_plus_plus_17:
+// FIXME: add a separate case for CPlusPlus14. Currently folded into C++17
+// because C++14 is the default standard for Clang but enabling CPlusPlus14
+// expression evaluatino doesn't pass the test-suite cleanly.
+lang_opts.CPlusPlus14 = true;
+lang_opts.CPlusPlus17 = true;
+LLVM_FALLTHROUGH;
   case lldb::eLanguageTypeC_plus_plus:
   case lldb::eLanguageTypeC_plus_plus_11:
   case lldb::eLanguageTypeC_plus_plus_14:

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index 527055024a76..68e25618f78b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -274,7 +274,7 @@ TokenVerifier::TokenVerifier(std::string body) {
   LangOptions Opts;
   Opts.ObjC = true;
   Opts.DollarIdents = true;
-  Opts.CPlusPlus17 = true;
+  Opts.CPlusPlus20 = true;
   Opts.LineComment = true;
 
   Lexer lex(FID, buf->getMemBufferRef(), SM, Opts);

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
index 1f7d8a1b8598..d8c095d6edeb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -750,6 +750,7 @@ static const clang::LangOptions &GetLangOptions() {
 g_options.CPlusPlus11 = true;
 g_options.CPlusPlus14 = true;
 g_options.CPlusPlus17 = true;
+g_options.CPlusPlus20 = true;
   });
   return g_options;
 }

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp 
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index b661ec445332..15747136d59c 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -399,6 +399,7 @@ bool TypeSystemClang::IsOperator(llvm::StringRef name,
 .Case("=", clang::OO_Equal)
 .Case("==", clang::OO_EqualEqual)
 .Case("<", clang::OO_Less)
+.Case("<=>", clang::OO_Spaceship)
 .Case("<<", clang::OO_LessLess)
 .Case("<<=", clang::OO_LessLessEqual)
 .Case("<=", clang::OO_LessEqual)
@@ -510,6 +511,9 @@ static void ParseLangArgs(LangOptions &Opts, InputKind IK, 
const char *triple) {
   Opts.C99 = Std.isC99();
   Opts.CPlusPlus = Std.isCPlusPlus();
   Opts.CPlusPlus11 = Std.isCPlusPlus11();
+  Opts.CPlusPlus14 = Std.isCPlusPlus14();
+  Opts.CPlusPlus17 = Std.isCPlusPlus17();
+  Opts.CPlusPlus20 = Std.isCPlusPlus20();
   Opts.Digraphs = Std.hasDigraphs();
   Opts.GNUMode = Std

[Lldb-commits] [PATCH] D143061: [lldb][Language] Add more language types

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0301a492f43e: [lldb][Language] Add more language types 
(authored by Michael137).

Changed prior to commit:
  https://reviews.llvm.org/D143061?vs=493896&id=513622#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143061

Files:
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Target/Language.cpp


Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -194,6 +194,21 @@
 {"c++14", eLanguageTypeC_plus_plus_14},
 {"fortran03", eLanguageTypeFortran03},
 {"fortran08", eLanguageTypeFortran08},
+{"renderscript", eLanguageTypeRenderScript},
+{"bliss", eLanguageTypeBLISS},
+{"kotlin", eLanguageTypeKotlin},
+{"zig", eLanguageTypeZig},
+{"crystal", eLanguageTypeCrystal},
+{"",
+ static_cast(
+ 0x0029)}, // Not yet taken by any language in the DWARF spec
+   // and thus has no entry in LanguageType
+{"c++17", eLanguageTypeC_plus_plus_17},
+{"c++20", eLanguageTypeC_plus_plus_20},
+{"c17", eLanguageTypeC17},
+{"fortran18", eLanguageTypeFortran18},
+{"ada2005", eLanguageTypeAda2005},
+{"ada2012", eLanguageTypeAda2012},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
 // Now synonyms, in arbitrary order
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -479,12 +479,24 @@
   eLanguageTypeC_plus_plus_14 = 0x0021, ///< ISO C++:2014.
   eLanguageTypeFortran03 = 0x0022,  ///< ISO Fortran 2003.
   eLanguageTypeFortran08 = 0x0023,  ///< ISO Fortran 2008.
+  eLanguageTypeRenderScript = 0x0024,
+  eLanguageTypeBLISS = 0x0025,
+  eLanguageTypeKotlin = 0x0026,
+  eLanguageTypeZig = 0x0027,
+  eLanguageTypeCrystal = 0x0028,
+  eLanguageTypeC_plus_plus_17 = 0x002a, ///< ISO C++:2017.
+  eLanguageTypeC_plus_plus_20 = 0x002b, ///< ISO C++:2020.
+  eLanguageTypeC17 = 0x002c,
+  eLanguageTypeFortran18 = 0x002d,
+  eLanguageTypeAda2005 = 0x002e,
+  eLanguageTypeAda2012 = 0x002f,
+
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler = 0x0024,   ///< Mips_Assembler.
+  eLanguageTypeMipsAssembler,   ///< Mips_Assembler.
   eNumLanguageTypes
 };
 


Index: lldb/source/Target/Language.cpp
===
--- lldb/source/Target/Language.cpp
+++ lldb/source/Target/Language.cpp
@@ -194,6 +194,21 @@
 {"c++14", eLanguageTypeC_plus_plus_14},
 {"fortran03", eLanguageTypeFortran03},
 {"fortran08", eLanguageTypeFortran08},
+{"renderscript", eLanguageTypeRenderScript},
+{"bliss", eLanguageTypeBLISS},
+{"kotlin", eLanguageTypeKotlin},
+{"zig", eLanguageTypeZig},
+{"crystal", eLanguageTypeCrystal},
+{"",
+ static_cast(
+ 0x0029)}, // Not yet taken by any language in the DWARF spec
+   // and thus has no entry in LanguageType
+{"c++17", eLanguageTypeC_plus_plus_17},
+{"c++20", eLanguageTypeC_plus_plus_20},
+{"c17", eLanguageTypeC17},
+{"fortran18", eLanguageTypeFortran18},
+{"ada2005", eLanguageTypeAda2005},
+{"ada2012", eLanguageTypeAda2012},
 // Vendor Extensions
 {"assembler", eLanguageTypeMipsAssembler},
 // Now synonyms, in arbitrary order
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -479,12 +479,24 @@
   eLanguageTypeC_plus_plus_14 = 0x0021, ///< ISO C++:2014.
   eLanguageTypeFortran03 = 0x0022,  ///< ISO Fortran 2003.
   eLanguageTypeFortran08 = 0x0023,  ///< ISO Fortran 2008.
+  eLanguageTypeRenderScript = 0x0024,
+  eLanguageTypeBLISS = 0x0025,
+  eLanguageTypeKotlin = 0x0026,
+  eLanguageTypeZig = 0x0027,
+  eLanguageTypeCrystal = 0x0028,
+  eLanguageTypeC_plus_plus_17 = 0x002a, ///< ISO C++:2017.
+  eLanguageTypeC_plus_plus_20 = 0x002b, ///< ISO C++:2020.
+  eLanguageTypeC17 = 0x002c,
+  eLanguageTypeFortran18 = 0x002d,
+  eLanguageTypeAda2005 = 0x002e,
+  eLanguageTypeAda2012 = 0x002f,
+
   // Vendor Extensions
   // Note: Language::GetNameForLanguageType
   // assumes these can be used as indexes into array language_names, and
   // Language::SetLanguageFromCString and Language::AsCString assume these can
   // be used as indexes into array g_languages.
-  eLanguageTypeMipsAssembler = 0x0024,   ///< Mips_A

[Lldb-commits] [lldb] 6cdfa29 - [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2023-04-14T17:11:30+01:00
New Revision: 6cdfa295743729178ff6f15a8dcd36f8f7d27c2c

URL: 
https://github.com/llvm/llvm-project/commit/6cdfa295743729178ff6f15a8dcd36f8f7d27c2c
DIFF: 
https://github.com/llvm/llvm-project/commit/6cdfa295743729178ff6f15a8dcd36f8f7d27c2c.diff

LOG: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

**Summary**

In a program such as:
```
namespace A {
namespace B {
struct Bar {};
}
}

namespace B {
struct Foo {};
}
```
...LLDB would run into issues such as:
```
(lldb) expr ::B::Foo f
error: expression failed to parse:
error: :1:6: no type named 'Foo' in namespace 'A::B'
::B::Foo f
~^
```

This is because the `SymbolFileDWARF::FindNamespace` implementation
will return *any* namespace it finds if the `parent_decl_ctx` provided
is empty. In `FindExternalVisibleDecls` we use this API to find the
namespace that symbol `B` refers to. If `A::B` happened to be the one
that `SymbolFileDWARF::FindNamespace` looked at first, we would try
to find `struct Foo` in `A::B`. Hence the error.

This patch proposes a new `SymbolFileDWARF::FindNamespace` API that
will only find a match for top-level namespaces, which is what
`FindExternalVisibleDecls` is attempting anyway; it just never
accounted for multiple namespaces of the same name.

**Testing**

* Added API test-case

Differential Revision: https://reviews.llvm.org/D147436

Added: 


Modified: 
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Symbol/SymbolFileOnDemand.h
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
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/SymbolFileOnDemand.cpp
lldb/test/API/lang/cpp/namespace/TestNamespace.py
lldb/test/API/lang/cpp/namespace/main.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/SymbolFile.h 
b/lldb/include/lldb/Symbol/SymbolFile.h
index 90162b7827d3..8de752816cf9 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -327,8 +327,17 @@ class SymbolFile : public PluginInterface {
   virtual llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) = 0;
 
+  /// Finds a namespace of name \ref name and whose parent
+  /// context is \ref parent_decl_ctx.
+  ///
+  /// If \code{.cpp} !parent_decl_ctx.IsValid() \endcode
+  /// then this function will consider all namespaces that
+  /// match the name. If \ref only_root_namespaces is
+  /// true, only consider in the search those DIEs that
+  /// represent top-level namespaces.
   virtual CompilerDeclContext
-  FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx) {
+  FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx,
+bool only_root_namespaces = false) {
 return CompilerDeclContext();
   }
 

diff  --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h 
b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
index 825fba755e99..adf1017ce73c 100644
--- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -171,9 +171,10 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile 
{
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private::CompilerDeclContext &parent_decl_ctx,
+bool only_root_namespaces) override;
 
   std::vector>
   ParseCallEdgesInFunction(UserID func_id) override;

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 108566b066d7..76cbfc4c05f8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -700,7 +700,18 @@ void ClangASTSource::FillNamespaceMap(
 if (!symbol_file)
   continue;
 
-found_namespace_decl = symbol_file->FindNamespace(name, namespace_decl);
+// If namespace_decl is not valid, 'FindNamespace' would look for
+// any namespace called 'name' (ignoring parent contexts) and return
+// the first one it finds. 

[Lldb-commits] [PATCH] D147436: [lldb][ClangExpression] Filter out non-root namespaces in FindNamespace

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6cdfa2957437: [lldb][ClangExpression] Filter out non-root 
namespaces in FindNamespace (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147436

Files:
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  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/SymbolFileOnDemand.cpp
  lldb/test/API/lang/cpp/namespace/TestNamespace.py
  lldb/test/API/lang/cpp/namespace/main.cpp

Index: lldb/test/API/lang/cpp/namespace/main.cpp
===
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,31 @@
 return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
 }
 
+namespace B {
+struct Bar {
+int x() { return 42; }
+};
+Bar bar;
+} // namespace B
+
+namespace A::B {
+struct Bar {
+int y() { return 137; }
+};
+} // namespace A::B
+
+namespace NS1::NS2 {
+struct Foo {
+int bar() { return -2; }
+};
+} // namespace NS1::NS2
+
+namespace NS2 {
+struct Foo {
+int bar() { return -3; }
+};
+} // namespace NS2
+
 int
 main (int argc, char const *argv[])
 {
@@ -112,5 +137,8 @@
 A::B::test_lookup_at_nested_ns_scope_after_using();
 test_lookup_before_using_directive();
 test_lookup_after_using_directive();
-return Foo::myfunc(12);
+::B::Bar bb;
+A::B::Bar ab;
+return Foo::myfunc(12) + bb.x() + ab.y() + NS1::NS2::Foo{}.bar() +
+   NS2::Foo{}.bar();
 }
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,11 @@
 
 self.expect("expression variadic_sum", patterns=[
 '\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
+self.expect_expr("::NS1::NS2::Foo{}.bar() == -2 && ::NS2::Foo{}.bar() == -3",
+ result_type="bool", result_value="true")
+# FIXME: C++ unqualified namespace lookups currently not supported when instantiating types.
+self.expect_expr("NS2::Foo{}.bar() == -3", result_type="bool", result_value="false")
+self.expect_expr("((::B::Bar*)&::B::bar)->x()", result_type="int", result_value="42")
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
 
 CompilerDeclContext
 SymbolFileOnDemand::FindNamespace(ConstString name,
-  const CompilerDeclContext &parent_decl_ctx) {
+  const CompilerDeclContext &parent_decl_ctx,
+  bool only_root_namespaces) {
   if (!m_debug_info_enabled) {
 LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
  __FUNCTION__, name);
-return SymbolFile::FindNamespace(name, parent_decl_ctx);
+return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
   }
-  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+  return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+only_root_namespaces);
 }
 
 std::vector>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language) override;
 
-  lldb_private::CompilerDeclContext FindNamespace(
-  lldb_private::ConstString name,
-  const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+  lldb_private::CompilerDeclContext
+  FindNamespace(lldb_private::ConstString name,
+const lldb_private

[Lldb-commits] [PATCH] D145574: [lldb] Read register fields from target XML

[David Spickett via Phabricator via lldb-commits]

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



Comment at: lldb/include/lldb/lldb-private-types.h:14
 
+#include "lldb/Target/RegisterFlags.h"
 #include "lldb/lldb-private.h"

MaskRay wrote:
> Is there a library layering violation?
> 
> I assume that `lldb/Target/*.h` files depend on 
> `lldb/include/lldb/lldb-private-types.h`, so 
> `lldb/include/lldb/lldb-private-types.h` cannot include `lldb/Target/*.h` 
> files...
It is and thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145574

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

[Lldb-commits] [PATCH] D143062: [lldb] Allow evaluating expressions in C++20 mode

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:518
+// because C++14 is the default standard for Clang but enabling CPlusPlus14
+// expression evaluatino doesn't pass the test-suite cleanly.
+lang_opts.CPlusPlus14 = true;

s/evaluatino/evaluation/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143062

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

[Lldb-commits] [PATCH] D148228: [lldb] Change some pointers to refs in register printing code

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148228

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

[Lldb-commits] [PATCH] D147669: PECOFF: consume errors properly

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

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

Great. Thanks!


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

https://reviews.llvm.org/D147669

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

[Lldb-commits] [PATCH] D147292: [lldb] Add support for the DW_AT_trampoline attribute with a boolean

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM with Adrian's and my comments addressed.




Comment at: lldb/include/lldb/Symbol/Function.h:439
   /// The section offset based address for this function.
+  /// \param[in] generic_trampoline
+  /// If this function is a generic trampoline. A generic trampoline 

aprantl wrote:
> Is the "generic" qualifier necessary here? If we later add support for 
> trampolines with a jump target maybe, but without that this seems to just 
> create opportunity for confusion, particularly for Swift where the word 
> "generic" has very different connotations.
I wondered that too, but assumed we were still planning to support trampolines 
with a target. If not, then +1 on making this just "trampoline". 



Comment at: lldb/source/Target/ThreadPlanStepRange.cpp:520-522
+  if (log)
+log->PutCString("ThreadPlanStepRange got asked if it explains the "
+"stop for some reason other than step.");

```
LLDB_LOG(GetLog(LLDBLog::Step), "ThreadPlanStepRange got asked if it explains 
the stop for some reason other than step.");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147292

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

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This class is not safe in multithreaded code. It's possible for one
thread to modify a PathMappingList's `m_pair` vector while another
thread is iterating over it, effectively invalidating the iterator and
potentially leading to crashes or other difficult-to-diagnose bugs.

rdar://107695786


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148380

Files:
  lldb/include/lldb/Target/PathMappingList.h
  lldb/source/Target/PathMappingList.cpp

Index: lldb/source/Target/PathMappingList.cpp
===
--- lldb/source/Target/PathMappingList.cpp
+++ lldb/source/Target/PathMappingList.cpp
@@ -48,6 +48,7 @@
 
 const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
   if (this != &rhs) {
+std::scoped_lock locks(m_mutex, rhs.m_mutex);
 m_pairs = rhs.m_pairs;
 m_callback = nullptr;
 m_callback_baton = nullptr;
@@ -60,6 +61,7 @@
 
 void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
  bool notify) {
+  std::lock_guard lock(m_mutex);
   ++m_mod_id;
   m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
   if (notify && m_callback)
@@ -67,6 +69,7 @@
 }
 
 void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
+  std::scoped_lock locks(m_mutex, rhs.m_mutex);
   ++m_mod_id;
   if (!rhs.m_pairs.empty()) {
 const_iterator pos, end = rhs.m_pairs.end();
@@ -81,6 +84,7 @@
llvm::StringRef replacement, bool notify) {
   auto normalized_path = NormalizePath(path);
   auto normalized_replacement = NormalizePath(replacement);
+  std::lock_guard lock(m_mutex);
   for (const auto &pair : m_pairs) {
 if (pair.first.GetStringRef().equals(normalized_path) &&
 pair.second.GetStringRef().equals(normalized_replacement))
@@ -92,6 +96,7 @@
 
 void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
  uint32_t index, bool notify) {
+  std::lock_guard lock(m_mutex);
   ++m_mod_id;
   iterator insert_iter;
   if (index >= m_pairs.size())
@@ -106,6 +111,7 @@
 
 bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
   uint32_t index, bool notify) {
+  std::lock_guard lock(m_mutex);
   if (index >= m_pairs.size())
 return false;
   ++m_mod_id;
@@ -116,6 +122,7 @@
 }
 
 bool PathMappingList::Remove(size_t index, bool notify) {
+  std::lock_guard lock(m_mutex);
   if (index >= m_pairs.size())
 return false;
 
@@ -130,6 +137,7 @@
 // For clients which do not need the pair index dumped, pass a pair_index >= 0
 // to only dump the indicated pair.
 void PathMappingList::Dump(Stream *s, int pair_index) {
+  std::lock_guard lock(m_mutex);
   unsigned int numPairs = m_pairs.size();
 
   if (pair_index < 0) {
@@ -147,6 +155,7 @@
 
 llvm::json::Value PathMappingList::ToJSON() {
   llvm::json::Array entries;
+  std::lock_guard lock(m_mutex);
   for (const auto &pair : m_pairs) {
 llvm::json::Array entry{pair.first.GetStringRef().str(),
 pair.second.GetStringRef().str()};
@@ -156,6 +165,7 @@
 }
 
 void PathMappingList::Clear(bool notify) {
+  std::lock_guard lock(m_mutex);
   if (!m_pairs.empty())
 ++m_mod_id;
   m_pairs.clear();
@@ -186,6 +196,7 @@
 
 std::optional PathMappingList::RemapPath(llvm::StringRef mapping_path,
bool only_if_exists) const {
+  std::lock_guard lock(m_mutex);
   if (m_pairs.empty() || mapping_path.empty())
 return {};
   LazyBool path_is_relative = eLazyBoolCalculate;
@@ -224,6 +235,7 @@
 PathMappingList::ReverseRemapPath(const FileSpec &file, FileSpec &fixed) const {
   std::string path = file.GetPath();
   llvm::StringRef path_ref(path);
+  std::lock_guard lock(m_mutex);
   for (const auto &it : m_pairs) {
 llvm::StringRef removed_prefix = it.second.GetStringRef();
 if (!path_ref.consume_front(it.second.GetStringRef()))
@@ -252,6 +264,7 @@
 
 bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
   bool notify) {
+  std::lock_guard lock(m_mutex);
   uint32_t idx = FindIndexForPath(path);
   if (idx < m_pairs.size()) {
 ++m_mod_id;
@@ -264,6 +277,7 @@
 }
 
 bool PathMappingList::Remove(ConstString path, bool notify) {
+  std::lock_guard lock(m_mutex);
   iterator pos = FindIteratorForPath(path);
   if (pos != m_pairs.end()) {
 ++m_mod_id;
@@ -277,6 +291,7 @@
 
 PathMappingList::const_iterator
 PathMappingList::FindIteratorForPath(ConstString path) const {
+  std::lock_guard lock(m_mutex);
   const_iterator pos;
   const_iterator begin = m_pairs.begin();
   const_it

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Does this actually have to be a //recursive// mutex?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D148380#4269862 , @JDevlieghere 
wrote:

> Does this actually have to be a //recursive// mutex?

Good point, I don't think it does. I'll update this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers updated this revision to Diff 513779.
nickdesaulniers added a comment.
This revision is now accepted and ready to land.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- fix lldb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/include/llvm/Demangle/Utility.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/ItaniumDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
  llvm/unittests/Demangle/ItaniumDemangleTest.cpp
  llvm/unittests/Demangle/OutputBufferTest.cpp

Index: llvm/unittests/Demangle/OutputBufferTest.cpp
===
--- llvm/unittests/Demangle/OutputBufferTest.cpp
+++ llvm/unittests/Demangle/OutputBufferTest.cpp
@@ -10,12 +10,13 @@
 #include "llvm/Demangle/Utility.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
Index: llvm/unittests/Demangle/ItaniumDemangleTest.cpp
===
--- llvm/unittests/Demangle/ItaniumDemangleTest.cpp
+++ llvm/unittests/Demangle/ItaniumDemangleTest.cpp
@@ -11,6 +11,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +84,7 @@
 }
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
@@ -98,7 +99,7 @@
   OutputBuffer OB;
   Node *N = AbstractManglingParser::parseType();
   N->printLeft(OB);
-  StringView Name = N->getBaseName();
+  std::string_view Name = N->getBaseName();
   if (!Name.empty())
 Types.push_back(std::string(Name.begin(), Name.end()));
   else
Index: llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
===
--- llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
+++ llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
@@ -17,13 +17,12 @@
 using llvm::itanium_demangle::ForwardTemplateReference;
 using llvm::itanium_demangle::Node;
 using llvm::itanium_demangle::NodeKind;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 struct FoldingSetNodeIDBuilder {
   llvm::FoldingSetNodeID &ID;
   void operator()(const Node *P) { ID.AddPointer(P); }
-  void operator()(StringView Str) {
+  void operator()(std::string_view Str) {
 ID.AddString(llvm::StringRef(Str.begin(), Str.size()));
   }
   template 
@@ -292,7 +291,7 @@
 N = Demangler.parse();
   else
 N = Demangler.make(
-StringView(Mangling.data(), Mangling.size()));
+std::string_view(Mangling.data(), Mangling.size()));
   return reinterpret_cast(N);
 }
 
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -11,8 +11,8 @@
 //
 //===--===//
 
+#include "llvm/ADT/StringViewExtras.h"
 #include "llvm/Demangle/Demangle.h"
-#include "llvm/Demangle/StringView.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -25,12 +25,11 @@
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 
 struct Identifier {
-  StringView Name;
+  std::string_view Name;
   bool Punycode;
 
   bool empty() const { return Name.empty(); }
@@ -77,7 +76,7 @@
   size_t RecursionLevel;
   size_t BoundLifetimes;
   // Input string that is being demangled with "_R" prefix removed.
-  StringView Input;
+  std::string_view Input;
   // Position in the input string.
   size_t Position;
   // When true, print methods append the output to the stream.
@@ -92,7 +91,7 @@
 
   Demangler(size_t MaxRecursionLevel = 500);
 
-  bool demangle(StringView MangledName);
+  bool demangle(std::string_view MangledName);
 
 private:
   bool demanglePath(IsInType Type,
@@ -128,10 +127,10 @@
   uint64_t parseOptionalBase62Number(char Tag);
   uint64_t parseBase62Number();
   uint64_t parseDecimalNumber();
-  uint64_t parseHexNumber(StringView &HexDigits);
+  uint64_t parseHexNumber(std::string_view &HexDigits);
 
   void print(char C);
-  void print(StringView S);
+  void print(std::string_view S);

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D148380#4269876 , @bulbazord wrote:

> In D148380#4269862 , @JDevlieghere 
> wrote:
>
>> Does this actually have to be a //recursive// mutex?
>
> Good point, I don't think it does. I'll update this.

Scratch that -- It unfortunately does need to be a recursive mutex. 
`PathMappingList` has a callback that may try to perform another operation on 
the list itself. This happens if you try to use `target modules search-paths 
add` -- We append to the list, the specified callback tries to read the size of 
the list. With a std::mutex we deadlock.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D148380#4270085 , @bulbazord wrote:

> In D148380#4269876 , @bulbazord 
> wrote:
>
>> In D148380#4269862 , @JDevlieghere 
>> wrote:
>>
>>> Does this actually have to be a //recursive// mutex?
>>
>> Good point, I don't think it does. I'll update this.
>
> Scratch that -- It unfortunately does need to be a recursive mutex. 
> `PathMappingList` has a callback that may try to perform another operation on 
> the list itself. This happens if you try to use `target modules search-paths 
> add` -- We append to the list, the specified callback tries to read the size 
> of the list. With a std::mutex we deadlock.

I see, and I assume it doesn't make sense to unlock the mutex before the 
callback?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [lldb] 3e55950 - [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via lldb-commits]

Author: Nick Desaulniers
Date: 2023-04-14T15:48:38-07:00
New Revision: 3e559509b426b6aae735a7f57dbdaed1041d2622

URL: 
https://github.com/llvm/llvm-project/commit/3e559509b426b6aae735a7f57dbdaed1041d2622
DIFF: 
https://github.com/llvm/llvm-project/commit/3e559509b426b6aae735a7f57dbdaed1041d2622.diff

LOG: [Demangle] replace use of llvm::StringView w/ std::string_view

This refactoring was waiting on converting LLVM to C++17.

Leave StringView.h and cleanup around for subsequent cleanup.

Reviewed By: MaskRay

Differential Revision: https://reviews.llvm.org/D148384

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
llvm/include/llvm/Demangle/ItaniumDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
llvm/include/llvm/Demangle/Utility.h
llvm/lib/Demangle/DLangDemangle.cpp
llvm/lib/Demangle/ItaniumDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
llvm/lib/Demangle/RustDemangle.cpp
llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
llvm/unittests/Demangle/ItaniumDemangleTest.cpp
llvm/unittests/Demangle/OutputBufferTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 1e9e1be62e3b5..71e4d566545e5 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -25,6 +25,7 @@
 #include "UdtRecordCompleter.h"
 #include "SymbolFileNativePDB.h"
 #include 
+#include 
 
 using namespace lldb_private;
 using namespace lldb_private::npdb;
@@ -174,7 +175,7 @@ PdbAstBuilder::CreateDeclInfoForType(const TagRecord 
&record, TypeIndex ti) {
 return CreateDeclInfoForUndecoratedName(record.Name);
 
   llvm::ms_demangle::Demangler demangler;
-  StringView sv(record.UniqueName.begin(), record.UniqueName.size());
+  std::string_view sv(record.UniqueName.begin(), record.UniqueName.size());
   llvm::ms_demangle::TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
   if (demangler.Error)
 return {m_clang.GetTranslationUnitDecl(), std::string(record.UniqueName)};

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 01756e47e917f..8c91033ba2604 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -57,6 +57,7 @@
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 #include 
+#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -631,7 +632,7 @@ static std::string GetUnqualifiedTypeName(const TagRecord 
&record) {
   }
 
   llvm::ms_demangle::Demangler demangler;
-  StringView sv(record.UniqueName.begin(), record.UniqueName.size());
+  std::string_view sv(record.UniqueName.begin(), record.UniqueName.size());
   llvm::ms_demangle::TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
   if (demangler.Error)
 return std::string(record.Name);

diff  --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h 
b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index 015c7105e6f05..05a21d9ec6255 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -16,8 +16,9 @@
 #ifndef DEMANGLE_ITANIUMDEMANGLE_H
 #define DEMANGLE_ITANIUMDEMANGLE_H
 
+#include 
+
 #include "DemangleConfig.h"
-#include "StringView.h"
 #include "Utility.h"
 #include 
 #include 
@@ -27,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -287,7 +289,7 @@ class Node {
   // implementation.
   virtual void printRight(OutputBuffer &) const {}
 
-  virtual StringView getBaseName() const { return StringView(); }
+  virtual std::string_view getBaseName() const { return {}; }
 
   // Silence compiler warnings, this dtor will never be called.
   virtual ~Node() = default;
@@ -346,10 +348,10 @@ struct NodeArrayNode : Node {
 
 class DotSuffix final : public Node {
   const Node *Prefix;
-  const StringView Suffix;
+  const std::string_view Suffix;
 
 public:
-  DotSuffix(const Node *Prefix_, StringView Suffix_)
+  DotSuffix(const Node *Prefix_, std::string_view Suffix_)
   : Node(KDotSuffix), Prefix(Prefix_), Suffix(Suffix_) {}
 
   template void match(Fn F) const { F(Prefix, Suffix); }
@@ -364,15 +366,15 @@ class DotSuffix final : public Node {
 
 class VendorExtQualType final : public Node {
   const Node *Ty;
-  StringView Ext;
+  std::string_view Ext;
   const Node *TA;
 
 public:
-  VendorExtQualType(const Node *Ty_, StringView Ext_, const Node *TA_)
+  VendorExtQualType(const Node *Ty_, std::string_view Ext_, const Node *TA_)
   : Node(KVendorEx

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e559509b426: [Demangle] replace use of llvm::StringView w/ 
std::string_view (authored by nickdesaulniers).
Herald added a subscriber: JDevlieghere.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  llvm/include/llvm/Demangle/ItaniumDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangle.h
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/include/llvm/Demangle/Utility.h
  llvm/lib/Demangle/DLangDemangle.cpp
  llvm/lib/Demangle/ItaniumDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Demangle/RustDemangle.cpp
  llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
  llvm/unittests/Demangle/ItaniumDemangleTest.cpp
  llvm/unittests/Demangle/OutputBufferTest.cpp

Index: llvm/unittests/Demangle/OutputBufferTest.cpp
===
--- llvm/unittests/Demangle/OutputBufferTest.cpp
+++ llvm/unittests/Demangle/OutputBufferTest.cpp
@@ -10,12 +10,13 @@
 #include "llvm/Demangle/Utility.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
Index: llvm/unittests/Demangle/ItaniumDemangleTest.cpp
===
--- llvm/unittests/Demangle/ItaniumDemangleTest.cpp
+++ llvm/unittests/Demangle/ItaniumDemangleTest.cpp
@@ -11,6 +11,7 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 using namespace llvm;
@@ -83,7 +84,7 @@
 }
 
 static std::string toString(OutputBuffer &OB) {
-  StringView SV = OB;
+  std::string_view SV = OB;
   return {SV.begin(), SV.end()};
 }
 
@@ -98,7 +99,7 @@
   OutputBuffer OB;
   Node *N = AbstractManglingParser::parseType();
   N->printLeft(OB);
-  StringView Name = N->getBaseName();
+  std::string_view Name = N->getBaseName();
   if (!Name.empty())
 Types.push_back(std::string(Name.begin(), Name.end()));
   else
Index: llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
===
--- llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
+++ llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
@@ -17,13 +17,12 @@
 using llvm::itanium_demangle::ForwardTemplateReference;
 using llvm::itanium_demangle::Node;
 using llvm::itanium_demangle::NodeKind;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 struct FoldingSetNodeIDBuilder {
   llvm::FoldingSetNodeID &ID;
   void operator()(const Node *P) { ID.AddPointer(P); }
-  void operator()(StringView Str) {
+  void operator()(std::string_view Str) {
 ID.AddString(llvm::StringRef(Str.begin(), Str.size()));
   }
   template 
@@ -292,7 +291,7 @@
 N = Demangler.parse();
   else
 N = Demangler.make(
-StringView(Mangling.data(), Mangling.size()));
+std::string_view(Mangling.data(), Mangling.size()));
   return reinterpret_cast(N);
 }
 
Index: llvm/lib/Demangle/RustDemangle.cpp
===
--- llvm/lib/Demangle/RustDemangle.cpp
+++ llvm/lib/Demangle/RustDemangle.cpp
@@ -11,8 +11,8 @@
 //
 //===--===//
 
+#include "llvm/ADT/StringViewExtras.h"
 #include "llvm/Demangle/Demangle.h"
-#include "llvm/Demangle/StringView.h"
 #include "llvm/Demangle/Utility.h"
 
 #include 
@@ -25,12 +25,11 @@
 
 using llvm::itanium_demangle::OutputBuffer;
 using llvm::itanium_demangle::ScopedOverride;
-using llvm::itanium_demangle::StringView;
 
 namespace {
 
 struct Identifier {
-  StringView Name;
+  std::string_view Name;
   bool Punycode;
 
   bool empty() const { return Name.empty(); }
@@ -77,7 +76,7 @@
   size_t RecursionLevel;
   size_t BoundLifetimes;
   // Input string that is being demangled with "_R" prefix removed.
-  StringView Input;
+  std::string_view Input;
   // Position in the input string.
   size_t Position;
   // When true, print methods append the output to the stream.
@@ -92,7 +91,7 @@
 
   Demangler(size_t MaxRecursionLevel = 500);
 
-  bool demangle(StringView MangledName);
+  bool demangle(std::string_view MangledName);
 
 private:
   bool demanglePath(IsInType Type,
@@ -128,10 +127,10 @@
   uint64_t parseOptionalBase62Number(char Tag);
   uint64_t parseBase62Number();
   uint64_t parseDecimalNumber();
-  uint64_t parseHexNumber(StringView &HexDigits);
+  uint64_t parseHexNumber(std::string_view &HexDigits);
 
   void 

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added a comment.

These windows buildbot failures are tough to make out: 
https://lab.llvm.org/buildbot/#/builders/127/builds/46749/steps/4/logs/stdio

  [2/1778] Building CXX object 
lib\Demangle\CMakeFiles\LLVMDemangle.dir\DLangDemangle.cpp.obj
  FAILED: lib/Demangle/CMakeFiles/LLVMDemangle.dir/DLangDemangle.cpp.obj 
  
C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe
  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS 
-D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS 
-D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Ilib\Demangle -IC:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle 
-Iinclude -IC:\b\slave\sanitizer-windows\llvm-project\llvm\include /DWIN32 
/D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Zi /Oi /bigobj 
/permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 
-wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 
-wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 
-wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 
-we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes 
/Folib\Demangle\CMakeFiles\LLVMDemangle.dir\DLangDemangle.cpp.obj 
/Fdlib\Demangle\CMakeFiles\LLVMDemangle.dir\LLVMDemangle.pdb /FS -c 
C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\DLangDemangle.cpp
  
C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/Demangle/Utility.h(111):
 error C2664: 'void *memcpy(void *,const void *,size_t)': cannot convert 
argument 2 from 'std::_String_view_iterator<_Traits>' to 'const void *'
  with
  [
  _Traits=std::char_traits
  ]
  
C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/Demangle/Utility.h(111):
 note: No user-defined-conversion operator available that can perform this 
conversion, or the operator cannot be called
  C:\Program Files (x86)\Microsoft Visual 
Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\include\vcruntime_string.h(43):
 note: see declaration of 'memcpy'

So something crazy is going on with `std::string_view::iterator` and implicit 
conversion to `void*`? Is this an issue in Windows C++17 support?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

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

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Nick Desaulniers via Phabricator via lldb-commits]

nickdesaulniers added a comment.

In D148384#4270106 , @nickdesaulniers 
wrote:

> These windows buildbot failures are tough to make out: 
> https://lab.llvm.org/buildbot/#/builders/127/builds/46749/steps/4/logs/stdio
>
>   [2/1778] Building CXX object 
> lib\Demangle\CMakeFiles\LLVMDemangle.dir\DLangDemangle.cpp.obj
>   FAILED: lib/Demangle/CMakeFiles/LLVMDemangle.dir/DLangDemangle.cpp.obj 
>   
> C:\PROGRA~2\MIB055~1\2019\PROFES~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe
>   /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
> -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE 
> -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 
> -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE 
> -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS 
> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\Demangle 
> -IC:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle -Iinclude 
> -IC:\b\slave\sanitizer-windows\llvm-project\llvm\include /DWIN32 /D_WINDOWS   
> /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Zi /Oi /bigobj /permissive- /W4 
> -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 
> -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 
> -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 
> -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 
> -w14062 -we4238 /Gw /MD /O2 /Ob2  /EHs-c- /GR- -UNDEBUG -std:c++17 
> /showIncludes 
> /Folib\Demangle\CMakeFiles\LLVMDemangle.dir\DLangDemangle.cpp.obj 
> /Fdlib\Demangle\CMakeFiles\LLVMDemangle.dir\LLVMDemangle.pdb /FS -c 
> C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\DLangDemangle.cpp
>   
> C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/Demangle/Utility.h(111):
>  error C2664: 'void *memcpy(void *,const void *,size_t)': cannot convert 
> argument 2 from 'std::_String_view_iterator<_Traits>' to 'const void *'
>   with
>   [
>   _Traits=std::char_traits
>   ]
>   
> C:\b\slave\sanitizer-windows\llvm-project\llvm\include\llvm/Demangle/Utility.h(111):
>  note: No user-defined-conversion operator available that can perform this 
> conversion, or the operator cannot be called
>   C:\Program Files (x86)\Microsoft Visual 
> Studio\2019\Professional\VC\Tools\MSVC\14.29.30133\include\vcruntime_string.h(43):
>  note: see declaration of 'memcpy'
>
> So something crazy is going on with `std::string_view::iterator` and implicit 
> conversion to `void*`? Is this an issue in Windows C++17 support?
>
> EDIT: also:
>
>   
> C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\MicrosoftDemangle.cpp(2403):
>  error C2678: binary '-': no operator found which takes a left-hand operand 
> of type 'std::_String_view_iterator<_Traits>' (or there is no acceptable 
> conversion)
>   
>   
> C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\MicrosoftDemangle.cpp(2380):
>  warning C4477: 'printf' : format string '%.*s' requires an argument of type 
> 'char *', but variadic argument 3 has type 
> 'std::_String_view_iterator<_Traits>'
>   
>   
> C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\MicrosoftDemangle.cpp(2388):
>  warning C4477: 'printf' : format string '%.*s' requires an argument of type 
> 'char *', but variadic argument 3 has type 
> 'std::_String_view_iterator<_Traits>'
>   
>   
> C:\b\slave\sanitizer-windows\llvm-project\llvm\lib\Demangle\ItaniumDemangle.cpp(82):
>  warning C4477: 'fprintf' : format string '%.*s' requires an argument of type 
> 'char *', but variadic argument 2 has type 
> 'std::_String_view_iterator<_Traits>'

Perhaps:

  diff --git a/llvm/lib/Demangle/ItaniumDemangle.cpp 
b/llvm/lib/Demangle/ItaniumDemangle.cpp
  index 4bc00c5144a4..7a7bcd36fc68 100644
  --- a/llvm/lib/Demangle/ItaniumDemangle.cpp
  +++ b/llvm/lib/Demangle/ItaniumDemangle.cpp
  @@ -79,7 +79,7 @@ struct DumpVisitor {
   
 void printStr(const char *S) { fprintf(stderr, "%s", S); }
 void print(std::string_view SV) {
  -fprintf(stderr, "\"%.*s\"", (int)SV.size(), SV.begin());
  +fprintf(stderr, "\"%.*s\"", (int)SV.size(), &*SV.begin());
 }
 void print(const Node *N) {
   if (N)
  diff --git a/llvm/lib/Demangle/MicrosoftDemangle.cpp 
b/llvm/lib/Demangle/MicrosoftDemangle.cpp
  index 15cb0e7d8864..5204c24164d6 100644
  --- a/llvm/lib/Demangle/MicrosoftDemangle.cpp
  +++ b/llvm/lib/Demangle/MicrosoftDemangle.cpp
  @@ -2377,7 +2377,7 @@ void Demangler::dumpBackReferences() {
   T->output(OB, OF_Default);
   
   std::string_view B = OB;
  -std::printf("  [%d] - %.*s\n", (int)I, (int)B.size(), B.begin());
  +std::printf("  [%d] - %.*s\n", (int)I, (int)B.size(), &*B.begin());
 }
 std::free(OB.getBuffer());
   
  @@ -2386,7 +2386,7 @@ void Demangler::dumpBackReferences() {
 std::printf("%d name backreferences\n", (int)Backrefs.NamesCount);
 for (size_t I = 0; I < Bac

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch is a simple refactor that unifies the default and hijack
listener methods and attributes between ProcessAttachInfo and
ProcessLaunchInfo.

These 2 classes are both derived from the ProcessInfo base class so this
patch moves the listeners attributes and getter/setter methods to the
base class.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148395

Files:
  lldb/include/lldb/Host/ProcessLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/Host/common/ProcessLaunchInfo.cpp
  lldb/source/Utility/ProcessInfo.cpp

Index: lldb/source/Utility/ProcessInfo.cpp
===
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -22,12 +22,14 @@
 using namespace lldb_private;
 
 ProcessInfo::ProcessInfo()
-: m_executable(), m_arguments(), m_environment(), m_arch() {}
+: m_executable(), m_arguments(), m_environment(), m_arch(), m_listener_sp(),
+  m_hijack_listener_sp(), m_passthrough_listener_sp() {}
 
 ProcessInfo::ProcessInfo(const char *name, const ArchSpec &arch,
  lldb::pid_t pid)
 : m_executable(name), m_arguments(), m_environment(), m_arch(arch),
-  m_pid(pid) {}
+  m_pid(pid), m_listener_sp(), m_hijack_listener_sp(),
+  m_passthrough_listener_sp() {}
 
 void ProcessInfo::Clear() {
   m_executable.Clear();
Index: lldb/source/Host/common/ProcessLaunchInfo.cpp
===
--- lldb/source/Host/common/ProcessLaunchInfo.cpp
+++ lldb/source/Host/common/ProcessLaunchInfo.cpp
@@ -31,8 +31,8 @@
 
 ProcessLaunchInfo::ProcessLaunchInfo()
 : ProcessInfo(), m_working_dir(), m_plugin_name(), m_flags(0),
-  m_file_actions(), m_pty(new PseudoTerminal), m_monitor_callback(nullptr),
-  m_listener_sp(), m_hijack_listener_sp() {}
+  m_file_actions(), m_pty(new PseudoTerminal), m_monitor_callback(nullptr) {
+}
 
 ProcessLaunchInfo::ProcessLaunchInfo(const FileSpec &stdin_file_spec,
  const FileSpec &stdout_file_spec,
Index: lldb/include/lldb/Utility/ProcessInfo.h
===
--- lldb/include/lldb/Utility/ProcessInfo.h
+++ lldb/include/lldb/Utility/ProcessInfo.h
@@ -97,6 +97,19 @@
 m_scripted_metadata_sp = metadata_sp;
   }
 
+  // Get and set the actual listener that will be used for the process events
+  lldb::ListenerSP GetListener() const { return m_listener_sp; }
+
+  void SetListener(const lldb::ListenerSP &listener_sp) {
+m_listener_sp = listener_sp;
+  }
+
+  lldb::ListenerSP GetHijackListener() const { return m_hijack_listener_sp; }
+
+  void SetHijackListener(const lldb::ListenerSP &listener_sp) {
+m_hijack_listener_sp = listener_sp;
+  }
+
 protected:
   FileSpec m_executable;
   std::string m_arg0; // argv[0] if supported. If empty, then use m_executable.
@@ -109,6 +122,8 @@
   ArchSpec m_arch;
   lldb::pid_t m_pid = LLDB_INVALID_PROCESS_ID;
   lldb::ScriptedMetadataSP m_scripted_metadata_sp = nullptr;
+  lldb::ListenerSP m_listener_sp = nullptr;
+  lldb::ListenerSP m_hijack_listener_sp = nullptr;
 };
 
 // ProcessInstanceInfo
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -122,8 +122,6 @@
 ProcessInfo::operator=(launch_info);
 SetProcessPluginName(launch_info.GetProcessPluginName());
 SetResumeCount(launch_info.GetResumeCount());
-SetListener(launch_info.GetListener());
-SetHijackListener(launch_info.GetHijackListener());
 m_detach_on_error = launch_info.GetDetachOnError();
   }
 
@@ -174,28 +172,13 @@
 return false;
   }
 
-  lldb::ListenerSP GetHijackListener() const { return m_hijack_listener_sp; }
-
-  void SetHijackListener(const lldb::ListenerSP &listener_sp) {
-m_hijack_listener_sp = listener_sp;
-  }
-
   bool GetDetachOnError() const { return m_detach_on_error; }
 
   void SetDetachOnError(bool enable) { m_detach_on_error = enable; }
 
-  // Get and set the actual listener that will be used for the process events
-  lldb::ListenerSP GetListener() const { return m_listener_sp; }
-
-  void SetListener(const lldb::ListenerSP &listener_sp) {
-m_listener_sp = listener_sp;
-  }
-
   lldb::ListenerSP GetListenerForProcess(Debugger &debugger);
 
 protected:
-  lldb::ListenerSP m_listener_sp;
-  lldb::ListenerSP m_hijack_listener_sp;
   std::string m_plugin_name;
   uint32_t m_resume_count = 0; // How many times do we resume after launching
   bool m_wait_for_launch = false;
Index: lldb/include/lldb/Host/ProcessLau

[Lldb-commits] [PATCH] D148380: [lldb] Lock accesses to PathMappingLists's internals

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D148380#4270090 , @JDevlieghere 
wrote:

> In D148380#4270085 , @bulbazord 
> wrote:
>
>> In D148380#4269876 , @bulbazord 
>> wrote:
>>
>>> In D148380#4269862 , 
>>> @JDevlieghere wrote:
>>>
 Does this actually have to be a //recursive// mutex?
>>>
>>> Good point, I don't think it does. I'll update this.
>>
>> Scratch that -- It unfortunately does need to be a recursive mutex. 
>> `PathMappingList` has a callback that may try to perform another operation 
>> on the list itself. This happens if you try to use `target modules 
>> search-paths add` -- We append to the list, the specified callback tries to 
>> read the size of the list. With a std::mutex we deadlock.
>
> I see, and I assume it doesn't make sense to unlock the mutex before the 
> callback?

I modified this patch locally to use `std::mutex` and release the mutex before 
invoking the callback. The test suite seems to like it, but it's still 
technically incorrect because of `AppendUnique`. Using std::mutex, we'd have to 
first lock the mutex, check to see if the given paths are already in `m_pairs`, 
and if they are not, unlock the mutex and call Append. If 2 things try to 
AppendUnique the same things at the same time and the stars align, they're 
going to Append the same thing twice (which is what happens without this patch 
anyway). We'd need to do some refactors if we wanted std::mutex to work 
correctly instead of std::recursive_mutex. That may be worth doing in the 
future, but I'm going to land this as-is for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148380

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

[Lldb-commits] [PATCH] D148397: [lldb/Utility] Add opt-in passthrough mode to event listeners

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: jingham, JDevlieghere, bulbazord.
mib added a project: LLDB.
Herald added a subscriber: emaste.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch augments lldb's event listeners with a new passthrough mode.

As the name suggests, this mode allows events to be passed to an
additional listener to perform event monitoring, without interferring
with the event life cycle.

One of our use case for this, is to be able to listen to public process
events while making sure the events will still be delivered to the
default process listener (the debugger listener in most cases).

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148397

Files:
  lldb/include/lldb/API/SBAttachInfo.h
  lldb/include/lldb/API/SBLaunchInfo.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Listener.h
  lldb/include/lldb/Utility/ProcessInfo.h
  lldb/source/API/SBAttachInfo.cpp
  lldb/source/API/SBLaunchInfo.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -35,7 +35,7 @@
 
 Listener::Listener(const char *name)
 : m_name(name), m_broadcasters(), m_broadcasters_mutex(), m_events(),
-  m_events_mutex() {
+  m_events_mutex(), m_is_passthrough() {
   Log *log = GetLog(LLDBLog::Object);
   if (log != nullptr)
 LLDB_LOGF(log, "%p Listener::Listener('%s')", static_cast(this),
@@ -302,7 +302,8 @@
   // to return it so it should be okay to get the next event off the queue
   // here - and it might be useful to do that in the "DoOnRemoval".
   lock.unlock();
-  event_sp->DoOnRemoval();
+  if (!m_is_passthrough)
+event_sp->DoOnRemoval();
 }
 return true;
   }
Index: lldb/source/Utility/Broadcaster.cpp
===
--- lldb/source/Utility/Broadcaster.cpp
+++ lldb/source/Utility/Broadcaster.cpp
@@ -228,6 +228,8 @@
   &m_broadcaster, event_type))
   return;
 hijacking_listener_sp->AddEvent(event_sp);
+if (m_passthrough_listener)
+  m_passthrough_listener->AddEvent(event_sp);
   } else {
 for (auto &pair : GetListeners()) {
   if (!(pair.second & event_type))
@@ -237,6 +239,8 @@
 continue;
 
   pair.first->AddEvent(event_sp);
+  if (m_passthrough_listener)
+m_passthrough_listener->AddEvent(event_sp);
 }
   }
 }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3193,6 +3193,8 @@
 // Since we didn't have a platform launch the process, launch it here.
 if (m_process_sp) {
   m_process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  m_process_sp->SetPassthroughListener(
+  launch_info.GetPassthroughListener());
   error = m_process_sp->Launch(launch_info);
 }
   }
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -423,6 +423,8 @@
 
 if (process_sp) {
   process_sp->HijackProcessEvents(launch_info.GetHijackListener());
+  process_sp->SetPassthroughListener(
+  launch_info.GetPassthroughListener());
 
   error = process_sp->ConnectRemote(connect_url.c_str());
   // Retry the connect remote one time...
@@ -515,6 +517,8 @@
   ListenerSP listener_sp = attach_info.GetHijackListener();
   if (listener_sp)
 process_sp->HijackProcessEvents(listener_sp);
+  process_sp->SetPassthroughListener(
+  attach_info.GetPassthroughListener());
   error = process_sp->Attach(attach_info);
 }
 
Index: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
===
--- lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -401,6 +401,8 @@
   attach_info.SetHijackListener(listener_sp);
 }
 process_sp->HijackProcessEvents(listener_sp);
+process_sp->SetPassthroughListener(
+attach_info.GetPassthroughListener());
 error = process_sp->Attach(attach_info);
   }
 }
@@ -458,6 +460,7 @@
   LLDB_LOG(log, "success

[Lldb-commits] [PATCH] D148399: [lldb] Improve logging for process state change (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, jingham, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch improves process state change logging messages to include to
process plugin name.

This is very useful when investigating interactions between different
types of process plugins. That comes very handy when investigating bugs
related to interactive scripted process debugging.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148399

Files:
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1053,14 +1053,19 @@
   std::lock_guard guard(m_exit_status_mutex);
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(
-  log, "Process::SetExitStatus (status=%i (0x%8.8x), description=%s%s%s)",
-  status, status, cstr ? "\"" : "", cstr ? cstr : "NULL", cstr ? "\"" : "");
+  LLDB_LOGF(log,
+"Process::SetExitStatus (plugin = %s status=%i (0x%8.8x), "
+"description=%s%s%s)",
+GetPluginName().data(), status, status, cstr ? "\"" : "",
+cstr ? cstr : "NULL", cstr ? "\"" : "");
 
   // We were already in the exited state
   if (m_private_state.GetValue() == eStateExited) {
-LLDB_LOGF(log, "Process::SetExitStatus () ignoring exit status because "
-   "state was already set to eStateExited");
+LLDB_LOGF(
+log,
+"Process::SetExitStatus (plugin = %s) ignoring exit status because "
+"state was already set to eStateExited",
+GetPluginName().data());
 return false;
   }
 
@@ -1312,8 +1317,9 @@
   }
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(log, "Process::SetPublicState (state = %s, restarted = %i)",
-StateAsCString(new_state), restarted);
+  LLDB_LOGF(log,
+"Process::SetPublicState (plugin = %s, state = %s, restarted = %i)",
+GetPluginName().data(), StateAsCString(new_state), restarted);
   const StateType old_state = m_public_state.GetValue();
   m_public_state.SetValue(new_state);
 
@@ -1323,15 +1329,18 @@
   if (!StateChangedIsExternallyHijacked()) {
 if (new_state == eStateDetached) {
   LLDB_LOGF(log,
-"Process::SetPublicState (%s) -- unlocking run lock for detach",
-StateAsCString(new_state));
+"Process::SetPublicState (plugin = %s, state = %s) -- "
+"unlocking run lock for detach",
+GetPluginName().data(), StateAsCString(new_state));
   m_public_run_lock.SetStopped();
 } else {
   const bool old_state_is_stopped = StateIsStoppedState(old_state, false);
   if ((old_state_is_stopped != new_state_is_stopped)) {
 if (new_state_is_stopped && !restarted) {
-  LLDB_LOGF(log, "Process::SetPublicState (%s) -- unlocking run lock",
-StateAsCString(new_state));
+  LLDB_LOGF(log,
+"Process::SetPublicState (plugin = %s, state = %s) -- "
+"unlocking run lock",
+GetPluginName().data(), StateAsCString(new_state));
   m_public_run_lock.SetStopped();
 }
   }
@@ -1341,10 +1350,14 @@
 
 Status Process::Resume() {
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(log, "Process::Resume -- locking run lock");
+  LLDB_LOGF(log, "Process::Resume (plugin = %s) -- locking run lock",
+GetPluginName().data());
   if (!m_public_run_lock.TrySetRunning()) {
 Status error("Resume request failed - process still running.");
-LLDB_LOGF(log, "Process::Resume: -- TrySetRunning failed, not resuming.");
+LLDB_LOGF(
+log,
+"Process::Resume (plugin = %s) -- TrySetRunning failed, not resuming.",
+GetPluginName().data());
 return error;
   }
   Status error = PrivateResume();
@@ -1420,7 +1433,8 @@
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process | LLDBLog::Unwind));
   bool state_changed = false;
 
-  LLDB_LOGF(log, "Process::SetPrivateState (%s)", StateAsCString(new_state));
+  LLDB_LOGF(log, "Process::SetPrivateState (plugin = %s, state = %s)",
+GetPluginName().data(), StateAsCString(new_state));
 
   std::lock_guard thread_guard(m_thread_list.GetMutex());
   std::lock_guard guard(m_private_state.GetMutex());
@@ -1461,15 +1475,19 @@
   if (!m_mod_id.IsLastResumeForUserExpression())
 m_mod_id.SetStopEventForLastNaturalStopID(event_sp);
   m_memory_cache.Clear();
-  LLDB_LOGF(log, "Process::SetPrivateState (%s) stop_id = %u",
-StateAsCString(new_state), m_mod_id.GetStopID());
+  LLDB_LOGF(
+  log,
+  "Process::SetPrivateState (plugin = %s, state = %s, stop_id = %u",
+  

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[NAKAMURA Takumi via Phabricator via lldb-commits]

chapuni added inline comments.



Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:19
 
+#include 
+

Odd layering...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

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

[Lldb-commits] [PATCH] D148400: [lldb] Fix bug to update process public run lock with process state

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: jingham, JDevlieghere.
Herald added a project: All.
mib requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch should address an issue that caused the process public run
lock to not be updated during a process launch/attach when the process
stops.

That caused the public run lock to report its state as running while the
process state is stopped. This prevents the users to interact with the
process (through the command line or via the SBAPI) since it's
considered still running.

To address that, this patch refactors the name of the internal hijack
listeners to a specific pattern `lldb.internal..hijack` that
are used to ensure that we've attached to or launched a process successfully.

Then, when updating the process public state, after updating the state
value, if the process is not hijacked externally, meaning if the process
doens't have a hijack listener that matches the internal hijack
listeners pattern, we can update the public run lock accordingly.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148400

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3157,8 +3157,8 @@
   // its own hijacking listener or if the process is created by the target
   // manually, without the platform).
   if (!launch_info.GetHijackListener())
-launch_info.SetHijackListener(
-Listener::MakeListener("lldb.Target.Launch.hijack"));
+launch_info.SetHijackListener(Listener::MakeListener(
+Process::GetLaunchSynchronousHijackListenerName().data()));
 
   // If we're not already connected to the process, and if we have a platform
   // that can launch a process for debugging, go ahead and do that here.
@@ -3336,8 +3336,8 @@
   ListenerSP hijack_listener_sp;
   const bool async = attach_info.GetAsync();
   if (!async) {
-hijack_listener_sp =
-Listener::MakeListener("lldb.Target.Attach.attach.hijack");
+hijack_listener_sp = Listener::MakeListener(
+Process::GetAttachSynchronousHijackListenerName().data());
 attach_info.SetHijackListener(hijack_listener_sp);
   }
 
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -401,6 +401,24 @@
   return class_name;
 }
 
+llvm::StringRef Process::GetAttachSynchronousHijackListenerName() {
+  static ConstString class_name(
+  "lldb.internal.Process.AttachSynchronous.hijack");
+  return class_name.GetCString();
+}
+
+llvm::StringRef Process::GetLaunchSynchronousHijackListenerName() {
+  static ConstString class_name(
+  "lldb.internal.Process.LaunchSynchronous.hijack");
+  return class_name.GetCString();
+}
+
+llvm::StringRef Process::GetResumeSynchronousHijackListenerName() {
+  static ConstString class_name(
+  "lldb.internal.Process.ResumeSynchronous.hijack");
+  return class_name.GetCString();
+}
+
 Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp)
 : Process(target_sp, listener_sp, UnixSignals::CreateForHost()) {
   // This constructor just delegates to the full Process constructor,
@@ -1368,8 +1386,6 @@
   return error;
 }
 
-static const char *g_resume_sync_name = "lldb.Process.ResumeSynchronous.hijack";
-
 Status Process::ResumeSynchronous(Stream *stream) {
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
   LLDB_LOGF(log, "Process::ResumeSynchronous -- locking run lock");
@@ -1380,7 +1396,7 @@
   }
 
   ListenerSP listener_sp(
-  Listener::MakeListener(g_resume_sync_name));
+  Listener::MakeListener(GetResumeSynchronousHijackListenerName().data()));
   HijackProcessEvents(listener_sp);
 
   Status error = PrivateResume();
@@ -1408,7 +1424,7 @@
   if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
 const char *hijacking_name = GetHijackingListenerName();
 if (hijacking_name &&
-strcmp(hijacking_name, g_resume_sync_name))
+strstr(hijacking_name, "lldb.internal") != hijacking_name)
   return true;
   }
   return false;
@@ -1418,7 +1434,8 @@
   if (IsHijackedForEvent(eBroadcastBitStateChanged)) {
 const char *hijacking_name = GetHijackingListenerName();
 if (hijacking_name &&
-strcmp(hijacking_name, g_resume_sync_name) == 0)
+strcmp(hijacking_name,
+   GetResumeSynchronousHijackListenerName().data()) == 0)
   return true;
   }
   return false;
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -369,6 +369,10 @@
 
   static ConstString &GetStaticBroadcasterClass();
 
+

[Lldb-commits] [PATCH] D148401: [lldb/API] Add convenience constructor for SBError (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added a reviewer: bulbazord.
mib added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch adds a new convience constructor to the SBError to initialize
it with a string message to avoid having to create the object and call
the `SetErrorString` method afterwards.

This is very handy to report errors from lldb scripted affordances.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148401

Files:
  lldb/include/lldb/API/SBError.h
  lldb/source/API/SBError.cpp


Index: lldb/source/API/SBError.cpp
===
--- lldb/source/API/SBError.cpp
+++ lldb/source/API/SBError.cpp
@@ -25,6 +25,12 @@
   m_opaque_up = clone(rhs.m_opaque_up);
 }
 
+SBError::SBError(const char *message) {
+  LLDB_INSTRUMENT_VA(this, message);
+
+  SetErrorString(message);
+}
+
 SBError::SBError(const lldb_private::Status &status)
 : m_opaque_up(new Status(status)) {
   LLDB_INSTRUMENT_VA(this, status);
Index: lldb/include/lldb/API/SBError.h
===
--- lldb/include/lldb/API/SBError.h
+++ lldb/include/lldb/API/SBError.h
@@ -23,6 +23,8 @@
 
   SBError(const lldb::SBError &rhs);
 
+  SBError(const char *message);
+
 #ifndef SWIG
   SBError(const lldb_private::Status &error);
 #endif


Index: lldb/source/API/SBError.cpp
===
--- lldb/source/API/SBError.cpp
+++ lldb/source/API/SBError.cpp
@@ -25,6 +25,12 @@
   m_opaque_up = clone(rhs.m_opaque_up);
 }
 
+SBError::SBError(const char *message) {
+  LLDB_INSTRUMENT_VA(this, message);
+
+  SetErrorString(message);
+}
+
 SBError::SBError(const lldb_private::Status &status)
 : m_opaque_up(new Status(status)) {
   LLDB_INSTRUMENT_VA(this, status);
Index: lldb/include/lldb/API/SBError.h
===
--- lldb/include/lldb/API/SBError.h
+++ lldb/include/lldb/API/SBError.h
@@ -23,6 +23,8 @@
 
   SBError(const lldb::SBError &rhs);
 
+  SBError(const char *message);
+
 #ifndef SWIG
   SBError(const lldb_private::Status &error);
 #endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.



Comment at: llvm/include/llvm/Demangle/ItaniumDemangle.h:19
 
+#include 
+

chapuni wrote:
> Odd layering...
Good catch. LLVMDemangle cannot depend on other LLVM libraries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

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

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 513817.
mib retitled this revision from "[lldb] Add an example of interactive scripted 
process debugging (NFC)" to "[lldb] Add an example of interactive scripted 
process debugging".
mib edited the summary of this revision.
mib added a comment.

Many changes:

- Fixed many bugs
- Added test case

-


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

https://reviews.llvm.org/D145297

Files:
  lldb/test/API/functionalities/interactive_scripted_process/Makefile
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/test/API/functionalities/interactive_scripted_process/main.cpp

Index: lldb/test/API/functionalities/interactive_scripted_process/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/main.cpp
@@ -0,0 +1,35 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void spawn_thread(int index) {
+  std::string name = "I'm thread " + std::to_string(index) + " !";
+  bool done = false;
+  std::string state = "Started execution!";
+  while (true) {
+if (done) // also break here
+  break;
+  }
+
+  state = "Stopped execution!";
+}
+
+int main() {
+  size_t num_threads = 10;
+  std::vector threads;
+
+  for (size_t i = 0; i < num_threads; i++) {
+threads.push_back(std::thread(spawn_thread, i));
+  }
+
+  std::cout << "Spawned " << threads.size() << " threads!"; // Break here
+
+  for (auto &t : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -0,0 +1,427 @@
+# Usage:
+# ./bin/lldb $LLVM/lldb/test/API/functionalities/interactive_scripted_process/main \
+#   -o "br set -p 'Break here'" \
+#   -o "command script import $LLVM/lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py" \
+#   -o "create_mux" \
+#   -o "create_sub" \
+#   -o "br set -p 'also break here'" -o 'continue'
+
+import os,json,struct,signal
+import time
+
+from threading import Thread
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class PassthruScriptedProcess(ScriptedProcess):
+driving_target = None
+driving_process = None
+
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args :
+ lldb.SBStructuredData, launched_driving_process=True):
+super().__init__(exe_ctx, args)
+
+self.driving_target = None
+self.driving_process = None
+
+self.driving_target_idx = args.GetValueForKey("driving_target_idx")
+if (self.driving_target_idx and self.driving_target_idx.IsValid()):
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
+idx = self.driving_target_idx.GetIntegerValue(42)
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeString:
+idx = int(self.driving_target_idx.GetStringValue(100))
+self.driving_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+
+if launched_driving_process:
+self.driving_process = self.driving_target.GetProcess()
+for driving_thread in self.driving_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"driving_target_idx" : idx,
+"thread_idx" : driving_thread.GetIndexID()
+}))
+
+self.threads[driving_thread.GetThreadID()] = PassthruScriptedThread(self, structured_data)
+
+for module in self.driving_target.modules:
+path = module.file.fullpath
+load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(self.driving_target)
+self.loaded_images.append({"path": path, "load_addr": load_addr})
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+mem_region = lldb.SBMemoryRegionInfo()
+error = self.driving_process.GetMemoryRegionInfo(addr, mem_region)
+if error.Fail():
+return None
+return mem_region
+
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+data = lldb.SBData()
+bytes_read = self.driving_process.ReadMemory(addr, size, error)
+
+if error.Fail():
+return data
+
+data.SetDataWithOwnership(error, bytes_read,
+  

[Lldb-commits] [PATCH] D145297: [lldb] Add an example of interactive scripted process debugging

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 513818.

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

https://reviews.llvm.org/D145297

Files:
  lldb/test/API/functionalities/interactive_scripted_process/Makefile
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  
lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
  lldb/test/API/functionalities/interactive_scripted_process/main.cpp

Index: lldb/test/API/functionalities/interactive_scripted_process/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/main.cpp
@@ -0,0 +1,35 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+
+void spawn_thread(int index) {
+  std::string name = "I'm thread " + std::to_string(index) + " !";
+  bool done = false;
+  std::string state = "Started execution!";
+  while (true) {
+if (done) // also break here
+  break;
+  }
+
+  state = "Stopped execution!";
+}
+
+int main() {
+  size_t num_threads = 10;
+  std::vector threads;
+
+  for (size_t i = 0; i < num_threads; i++) {
+threads.push_back(std::thread(spawn_thread, i));
+  }
+
+  std::cout << "Spawned " << threads.size() << " threads!"; // Break here
+
+  for (auto &t : threads) {
+if (t.joinable())
+  t.join();
+  }
+
+  return 0;
+}
Index: lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
===
--- /dev/null
+++ lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py
@@ -0,0 +1,426 @@
+# Usage:
+# ./bin/lldb $LLVM/lldb/test/API/functionalities/interactive_scripted_process/main \
+#   -o "br set -p 'Break here'" \
+#   -o "command script import $LLVM/lldb/test/API/functionalities/interactive_scripted_process/interactive_scripted_process.py" \
+#   -o "create_mux" \
+#   -o "create_sub" \
+#   -o "br set -p 'also break here'" -o 'continue'
+
+import os,json,struct,signal
+import time
+
+from threading import Thread
+from typing import Any, Dict
+
+import lldb
+from lldb.plugins.scripted_process import ScriptedProcess
+from lldb.plugins.scripted_process import ScriptedThread
+
+class PassthruScriptedProcess(ScriptedProcess):
+driving_target = None
+driving_process = None
+
+def __init__(self, exe_ctx: lldb.SBExecutionContext, args :
+ lldb.SBStructuredData, launched_driving_process=True):
+super().__init__(exe_ctx, args)
+
+self.driving_target = None
+self.driving_process = None
+
+self.driving_target_idx = args.GetValueForKey("driving_target_idx")
+if (self.driving_target_idx and self.driving_target_idx.IsValid()):
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeInteger:
+idx = self.driving_target_idx.GetIntegerValue(42)
+if self.driving_target_idx.GetType() == lldb.eStructuredDataTypeString:
+idx = int(self.driving_target_idx.GetStringValue(100))
+self.driving_target = self.target.GetDebugger().GetTargetAtIndex(idx)
+
+if launched_driving_process:
+self.driving_process = self.driving_target.GetProcess()
+for driving_thread in self.driving_process:
+structured_data = lldb.SBStructuredData()
+structured_data.SetFromJSON(json.dumps({
+"driving_target_idx" : idx,
+"thread_idx" : driving_thread.GetIndexID()
+}))
+
+self.threads[driving_thread.GetThreadID()] = PassthruScriptedThread(self, structured_data)
+
+for module in self.driving_target.modules:
+path = module.file.fullpath
+load_addr = module.GetObjectFileHeaderAddress().GetLoadAddress(self.driving_target)
+self.loaded_images.append({"path": path, "load_addr": load_addr})
+
+def get_memory_region_containing_address(self, addr: int) -> lldb.SBMemoryRegionInfo:
+mem_region = lldb.SBMemoryRegionInfo()
+error = self.driving_process.GetMemoryRegionInfo(addr, mem_region)
+if error.Fail():
+return None
+return mem_region
+
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+data = lldb.SBData()
+bytes_read = self.driving_process.ReadMemory(addr, size, error)
+
+if error.Fail():
+return data
+
+data.SetDataWithOwnership(error, bytes_read,
+  self.driving_target.GetByteOrder(),
+  self.driving_target.GetAddressByteSize())
+
+return data
+
+def write_memory_at_address(self, addr, data, error):
+return self.driving_process.WriteMemory(addr,
+ 

[Lldb-commits] [PATCH] D148402: [lldb] Remove use of ConstString from Args::GetShellSafeArgument

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Having the names of various shells in ConstString's StringPool is not
really necessary, especially if they are otherwise not going to be there
in the first place. For example, if the person debugging uses bash on
their system, the `shell` parameter will have its `m_filename` set to a
ConstString containing "bash". However, fish, tcsh, zsh, and sh will
probably never be used and are just taking up space in the StringPool.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148402

Files:
  lldb/source/Utility/Args.cpp


Index: lldb/source/Utility/Args.cpp
===
--- lldb/source/Utility/Args.cpp
+++ lldb/source/Utility/Args.cpp
@@ -7,7 +7,6 @@
 
//===--===//
 
 #include "lldb/Utility/Args.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
@@ -385,20 +384,21 @@
 std::string Args::GetShellSafeArgument(const FileSpec &shell,
llvm::StringRef unsafe_arg) {
   struct ShellDescriptor {
-ConstString m_basename;
+llvm::StringRef m_basename;
 llvm::StringRef m_escapables;
   };
 
-  static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&;"},
-   {ConstString("fish"), " '\"<>()&\\|;"},
-   {ConstString("tcsh"), " '\"<>()&;"},
-   {ConstString("zsh"), " '\"<>()&;\\|"},
-   {ConstString("sh"), " '\"<>()&;"}};
+  static ShellDescriptor g_Shells[] = {{"bash", " '\"<>()&;"},
+   {"fish", " '\"<>()&\\|;"},
+   {"tcsh", " '\"<>()&;"},
+   {"zsh", " '\"<>()&;\\|"},
+   {"sh", " '\"<>()&;"}};
 
   // safe minimal set
   llvm::StringRef escapables = " '\"";
 
-  if (auto basename = shell.GetFilename()) {
+  auto basename = shell.GetFilename().GetStringRef();
+  if (!basename.empty()) {
 for (const auto &Shell : g_Shells) {
   if (Shell.m_basename == basename) {
 escapables = Shell.m_escapables;


Index: lldb/source/Utility/Args.cpp
===
--- lldb/source/Utility/Args.cpp
+++ lldb/source/Utility/Args.cpp
@@ -7,7 +7,6 @@
 //===--===//
 
 #include "lldb/Utility/Args.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
@@ -385,20 +384,21 @@
 std::string Args::GetShellSafeArgument(const FileSpec &shell,
llvm::StringRef unsafe_arg) {
   struct ShellDescriptor {
-ConstString m_basename;
+llvm::StringRef m_basename;
 llvm::StringRef m_escapables;
   };
 
-  static ShellDescriptor g_Shells[] = {{ConstString("bash"), " '\"<>()&;"},
-   {ConstString("fish"), " '\"<>()&\\|;"},
-   {ConstString("tcsh"), " '\"<>()&;"},
-   {ConstString("zsh"), " '\"<>()&;\\|"},
-   {ConstString("sh"), " '\"<>()&;"}};
+  static ShellDescriptor g_Shells[] = {{"bash", " '\"<>()&;"},
+   {"fish", " '\"<>()&\\|;"},
+   {"tcsh", " '\"<>()&;"},
+   {"zsh", " '\"<>()&;\\|"},
+   {"sh", " '\"<>()&;"}};
 
   // safe minimal set
   llvm::StringRef escapables = " '\"";
 
-  if (auto basename = shell.GetFilename()) {
+  auto basename = shell.GetFilename().GetStringRef();
+  if (!basename.empty()) {
 for (const auto &Shell : g_Shells) {
   if (Shell.m_basename == basename) {
 escapables = Shell.m_escapables;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Daniel Hoekwater via Phabricator via lldb-commits]

dhoekwater added a comment.

It looks like this breaks the build: 
https://lab.llvm.org/buildbot#builders/119/builds/12865 
https://lab.llvm.org/buildbot#builders/123/builds/18388 
https://lab.llvm.org/buildbot#builders/60/builds/11628

I'm pretty new with LLVM so I'll leave reverting the change to someone else, 
but I figured I should give you a heads up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

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

[Lldb-commits] [PATCH] D148395: [lldb] Unify default/hijack listener between Process{Attach, Launch}Info (NFC)

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

Creating a ProcessAttachInfo from a ProcessLaunchInfo with this change means 
they'll have different listeners. Is that ProcessLaunchInfo kept around for any 
other reason? Or is it just made to create the ProcessAttachInfo? This seems 
like a reasonable move to me, but I'm not sure how LaunchInfo and AttachInfo 
might interact.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148395

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

[Lldb-commits] [PATCH] D148384: [Demangle] replace use of llvm::StringView w/ std::string_view

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D148384#4270505 , @dhoekwater 
wrote:

> It looks like this breaks the build: 
> https://lab.llvm.org/buildbot#builders/119/builds/12865 
> https://lab.llvm.org/buildbot#builders/123/builds/18388 
> https://lab.llvm.org/buildbot#builders/60/builds/11628
>
> I'm pretty new with LLVM so I'll leave reverting the change to someone else, 
> but I figured I should give you a heads up.

I am unsure about the Windows issue and find it difficult to repair...   I'm 
testing a revert commit to not leave issues to the weekend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148384

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

[Lldb-commits] [lldb] d81cdb4 - Revert D148384 "[Demangle] replace use of llvm::StringView w/ std::string_view"

[Fangrui Song via lldb-commits]

Author: Fangrui Song
Date: 2023-04-14T18:42:11-07:00
New Revision: d81cdb49d74064e88843733e7da92db865943509

URL: 
https://github.com/llvm/llvm-project/commit/d81cdb49d74064e88843733e7da92db865943509
DIFF: 
https://github.com/llvm/llvm-project/commit/d81cdb49d74064e88843733e7da92db865943509.diff

LOG: Revert D148384 "[Demangle] replace use of llvm::StringView w/ 
std::string_view"

This reverts commit 3e559509b426b6aae735a7f57dbdaed1041d2622 and 
e0c4ffa796b553fa78c638a9584c05ac21fe07d5.

This still breaks Windows builds.

In addition, `#include ` in
llvm/include/llvm/Demangle/ItaniumDemangle.h is a library layering violation
(LLVMDemangle is the lowest LLVM library and cannot depend on LLVMSupport).

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
llvm/include/llvm/Demangle/ItaniumDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangle.h
llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
llvm/include/llvm/Demangle/Utility.h
llvm/lib/Demangle/DLangDemangle.cpp
llvm/lib/Demangle/ItaniumDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
llvm/lib/Demangle/RustDemangle.cpp
llvm/lib/ProfileData/ItaniumManglingCanonicalizer.cpp
llvm/unittests/Demangle/ItaniumDemangleTest.cpp
llvm/unittests/Demangle/OutputBufferTest.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
index 71e4d566545e5..1e9e1be62e3b5 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
@@ -25,7 +25,6 @@
 #include "UdtRecordCompleter.h"
 #include "SymbolFileNativePDB.h"
 #include 
-#include 
 
 using namespace lldb_private;
 using namespace lldb_private::npdb;
@@ -175,7 +174,7 @@ PdbAstBuilder::CreateDeclInfoForType(const TagRecord 
&record, TypeIndex ti) {
 return CreateDeclInfoForUndecoratedName(record.Name);
 
   llvm::ms_demangle::Demangler demangler;
-  std::string_view sv(record.UniqueName.begin(), record.UniqueName.size());
+  StringView sv(record.UniqueName.begin(), record.UniqueName.size());
   llvm::ms_demangle::TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
   if (demangler.Error)
 return {m_clang.GetTranslationUnitDecl(), std::string(record.UniqueName)};

diff  --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp 
b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index 8c91033ba2604..01756e47e917f 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -57,7 +57,6 @@
 #include "PdbUtil.h"
 #include "UdtRecordCompleter.h"
 #include 
-#include 
 
 using namespace lldb;
 using namespace lldb_private;
@@ -632,7 +631,7 @@ static std::string GetUnqualifiedTypeName(const TagRecord 
&record) {
   }
 
   llvm::ms_demangle::Demangler demangler;
-  std::string_view sv(record.UniqueName.begin(), record.UniqueName.size());
+  StringView sv(record.UniqueName.begin(), record.UniqueName.size());
   llvm::ms_demangle::TagTypeNode *ttn = demangler.parseTagUniqueName(sv);
   if (demangler.Error)
 return std::string(record.Name);

diff  --git a/llvm/include/llvm/Demangle/ItaniumDemangle.h 
b/llvm/include/llvm/Demangle/ItaniumDemangle.h
index 05a21d9ec6255..a0029b94815dc 100644
--- a/llvm/include/llvm/Demangle/ItaniumDemangle.h
+++ b/llvm/include/llvm/Demangle/ItaniumDemangle.h
@@ -16,9 +16,8 @@
 #ifndef DEMANGLE_ITANIUMDEMANGLE_H
 #define DEMANGLE_ITANIUMDEMANGLE_H
 
-#include 
-
 #include "DemangleConfig.h"
+#include "StringView.h"
 #include "Utility.h"
 #include 
 #include 
@@ -28,7 +27,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -289,7 +287,7 @@ class Node {
   // implementation.
   virtual void printRight(OutputBuffer &) const {}
 
-  virtual std::string_view getBaseName() const { return {}; }
+  virtual StringView getBaseName() const { return StringView(); }
 
   // Silence compiler warnings, this dtor will never be called.
   virtual ~Node() = default;
@@ -348,10 +346,10 @@ struct NodeArrayNode : Node {
 
 class DotSuffix final : public Node {
   const Node *Prefix;
-  const std::string_view Suffix;
+  const StringView Suffix;
 
 public:
-  DotSuffix(const Node *Prefix_, std::string_view Suffix_)
+  DotSuffix(const Node *Prefix_, StringView Suffix_)
   : Node(KDotSuffix), Prefix(Prefix_), Suffix(Suffix_) {}
 
   template void match(Fn F) const { F(Prefix, Suffix); }
@@ -366,15 +364,15 @@ class DotSuffix final : public Node {
 
 class VendorExtQualType final : public Node {
   const Node *Ty;
-  std::string_view Ext;
+  StringView Ext;
   const Node *TA;
 
 public:
-  VendorExtQualType(const Node *T