[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb924c8c71daa: [clang][LTO] Remove the use of `--` for arange 
option (authored by Qiongsi Wu qiongs...@gmail.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135400

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options-aranges.c


Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 
2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: "-plugin-opt=-generate-arange-section"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: "-plugin-opt=-generate-arange-section"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:392-443
+  // For anonymous unions in a struct, msvc generated pdb doesn't have the
+  // entity for that union. So, we need to construct anonymous union and struct
+  // based on field offsets. The final AST is likely not matching the exact
+  // original AST, but the memory layout is preseved.
+
+  // The end offset to a field/struct/union that ends at the offset.
+  std::map end_offset_map;

labath wrote:
> This could use a high-level explanation of the algorithm. Like, I know it 
> tries to stuff the fields into structs and unions, but I wasn't able to 
> figure out how it does that, and what are the operating invariants.
> 
> The thing I like about this algorithm, is that the most complicated part (the 
> thing I highlighted) is basically just playing with numbers and it is 
> independent of any complex objects and state. If this part is separated out 
> into a separate function, then it would be perfectly suitable for unit 
> testing, and the unit tests could also serve as documentation/examples of the 
> kinds of transformations that the algorithm does.
Moved into a separate function and added unit tests.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:64
+uint64_t base_offset;
+llvm::SmallVector fields;
+

labath wrote:
> Can we drop the `SP` part? I think that the owning references (I guess that's 
> this field) could be just plain Field instances (unique_ptr at worst), 
> and the rest could just be plain pointers and references.
The Field object is shared between fields and m_fields. And we can't have Field 
member instance inside Field class.



Comment at: lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp:53-59
+// CHECK-NEXT: | |-DefinitionData pass_in_registers aggregate standard_layout 
trivially_copyable pod trivial literal
+// CHECK-NEXT: | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | |-CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | |-CopyAssignment simple trivial has_const_param 
needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | `-Destructor simple irrelevant trivial needs_implicit

labath wrote:
> I think we should exclude all of these DefinitionData fields from the test 
> expectation, as they are largely irrelevant to the test (and they also 
> obfuscate it very well). Otherwise, people will have to update these whenever 
> a new field gets added.
> 
> I don't know whether it contains the thing you wanted to test (as I don't 
> know what that is), but the `type lookup C` output will contain the general 
> structure of the type, and it will be a lot more readable.
Thanks, I didn't know that before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 465933.
zequanwu marked 5 inline comments as done.
zequanwu added a comment.

Add unittests and address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h
  lldb/test/Shell/SymbolFile/NativePDB/Inputs/class_layout.lldbinit
  lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/global-classes.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
  lldb/test/Shell/SymbolFile/NativePDB/tag-types.cpp
  lldb/unittests/SymbolFile/NativePDB/CMakeLists.txt
  lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp

Index: lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
===
--- /dev/null
+++ lldb/unittests/SymbolFile/NativePDB/UdtRecordCompleterTests.cpp
@@ -0,0 +1,178 @@
+//===-- UdtRecordCompleterTests.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private::npdb;
+using namespace llvm;
+
+namespace {
+using Member = UdtRecordCompleter::Member;
+using Record = UdtRecordCompleter::Record;
+using MemberSP = std::shared_ptr;
+
+class UdtRecordCompleterRecordTests : public testing::Test {
+  Record record;
+
+public:
+  void SetKind(Member::Kind kind) { record.record.kind = kind; }
+  void CollectMember(StringRef name, uint64_t byte_offset, uint64_t byte_size) {
+record.CollectMember(name, byte_offset * 8, byte_size * 8,
+ clang::QualType(), lldb::eAccessPublic, 0);
+  }
+  void ConstructRecord() { record.ConstructRecord(); }
+
+  static bool VerifyMembers(const llvm::SmallVector ,
+const llvm::SmallVector ) {
+if (lhs.size() != rhs.size())
+  return false;
+for (size_t i = 0; i < lhs.size(); ++i) {
+  if (lhs[i]->kind != rhs[i]->kind || lhs[i]->name != rhs[i]->name ||
+  lhs[i]->bit_offset != rhs[i]->bit_offset ||
+  lhs[i]->bit_size != rhs[i]->bit_size ||
+  lhs[i]->base_offset != rhs[i]->base_offset ||
+  !VerifyMembers(lhs[i]->fields, rhs[i]->fields))
+return false;
+}
+return true;
+  }
+  bool VerifyRecord(Record ) {
+return record.start_offset == testRecord.start_offset &&
+   VerifyMembers(record.record.fields, testRecord.record.fields);
+  }
+};
+MemberSP AddField(Member *member, StringRef name, uint64_t byte_offset,
+  uint64_t byte_size, Member::Kind kind,
+  uint64_t base_offset = 0) {
+  auto field =
+  std::make_shared(name, byte_offset * 8, byte_size * 8,
+   clang::QualType(), lldb::eAccessPublic, 0);
+  field->kind = kind;
+  field->base_offset = base_offset;
+  member->fields.push_back(field);
+  return field;
+}
+} // namespace
+
+TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInStruct) {
+  SetKind(Member::Kind::Struct);
+  CollectMember("m1", 0, 4);
+  CollectMember("m2", 0, 4);
+  CollectMember("m3", 0, 1);
+  CollectMember("m4", 0, 8);
+  ConstructRecord();
+
+  // struct {
+  //   union {
+  //   m1;
+  //   m2;
+  //   m3;
+  //   m4;
+  //   };
+  // };
+  Record record;
+  record.start_offset = 0;
+  MemberSP u = AddField(, "", 0, 0, Member::Union);
+  AddField(u.get(), "m1", 0, 4, Member::Field);
+  AddField(u.get(), "m2", 0, 4, Member::Field);
+  AddField(u.get(), "m3", 0, 1, Member::Field);
+  AddField(u.get(), "m4", 0, 8, Member::Field);
+  EXPECT_TRUE(VerifyRecord(record));
+}
+
+TEST_F(UdtRecordCompleterRecordTests, TestAnonymousUnionInUnion) {
+  SetKind(Member::Kind::Union);
+  CollectMember("m1", 0, 4);
+  CollectMember("m2", 0, 4);
+  CollectMember("m3", 0, 1);
+  CollectMember("m4", 0, 8);
+  ConstructRecord();
+
+  // union {
+  //   m1;
+  //   m2;
+  //   m3;
+  //   m4;
+  // };
+  Record record;
+  record.start_offset = 0;
+  AddField(, "m1", 0, 4, Member::Field);
+  AddField(, "m2", 0, 4, Member::Field);
+  AddField(, "m3", 0, 1, Member::Field);
+  AddField(, "m4", 0, 8, Member::Field);
+  EXPECT_TRUE(VerifyRecord(record));
+}
+
+TEST_F(UdtRecordCompleterRecordTests, TestNestedUnionStructInStruct) {
+  SetKind(Member::Kind::Struct);
+  

[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:68
   // Lambda
-  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() 
const",
-   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()", 
"()", "const",
+  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() "
+   "const",

`git clang-format`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135413

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

[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

[Michael Buch via Phabricator via lldb-commits]

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

**Summary**

The primary motivation for this patch is to make sure we handle
the step-in behaviour for functions in the `std` namespace which
have an `auto` return type. Currently the default `step-avoid-regex`
setting is `^std::` but LLDB will still step into template functions
with `auto` return types in the `std` namespace.

**Details**
When we hit a breakpoint and check whether we should stop, we call
into `ThreadPlanStepInRange::FrameMatchesAvoidCriteria`. We then ask
for the frame function name via 
`SymbolContext::GetFunctionName(Mangled::ePreferDemangledWithoutArguments)`.
This ends up trying to parse the function name using 
`CPlusPlusLanguage::MethodName::GetBasename` which
parses the raw demangled name string.

In `CPlusPlusNameParser::ParseFunctionImpl` calls `ConsumeTypename` to skip
the (in our case auto) return type of the demangled name (according to the
Itanium ABI this is a valid thing to encode into the mangled name).

However, `ConsumeTypename` doesn't strip out a plain `auto` identifier
(it will strip a `decltype(auto) return type though)

This is another case where the `CPlusPlusNameParser` breaks us in subtle ways
due to evolving C++ syntax. There are longer-term plans of replacing the 
hand-rolled
C++ parser with an alternative that uses the mangle tree API to do the parsing 
for us.

**Testing**

- Added API and unit-tests
- Adding support for ABI tags into the parser is a larger undertaking which we 
would rather solve properly by using libcxxabi's mangle tree parser


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135413

Files:
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
  lldb/test/API/functionalities/step-avoids-regexp/Makefile
  lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
  lldb/test/API/functionalities/step-avoids-regexp/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
@@ -65,8 +65,10 @@
"XX::(anonymous namespace)::anon_class::anon_func"},
 
   // Lambda
-  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() const",
-   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()", "()", "const",
+  {"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()() "
+   "const",
+   "main::{lambda()#1}::operator()() const::{lambda()#1}", "operator()",
+   "()", "const",
"main::{lambda()#1}::operator()() const::{lambda()#1}::operator()"},
 
   // Function pointers
@@ -110,8 +112,15 @@
"f, sizeof(B)", "()", "",
"f, sizeof(B)"},
   {"llvm::Optional::operator*() const volatile &&",
-   "llvm::Optional", "operator*", "()", "const volatile &&",
-   "llvm::Optional::operator*"}};
+   "llvm::Optional", "operator*", "()",
+   "const volatile &&", "llvm::Optional::operator*"},
+
+  // auto return type
+  {"auto std::test_return_auto() const", "std",
+   "test_return_auto", "()", "const", "std::test_return_auto"},
+  {"decltype(auto) std::test_return_auto(int) const", "std",
+   "test_return_auto", "(int)", "const",
+   "std::test_return_auto"}};
 
   for (const auto  : test_cases) {
 CPlusPlusLanguage::MethodName method(ConstString(test.input));
Index: lldb/test/API/functionalities/step-avoids-regexp/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/step-avoids-regexp/main.cpp
@@ -0,0 +1,18 @@
+#include 
+
+namespace ignore {
+template  auto auto_ret(T x) { return 0; }
+[[gnu::abi_tag("test")]] int with_tag() { return 0; }
+template  [[gnu::abi_tag("test")]] int with_tag_template() {
+  return 0;
+}
+
+template  decltype(auto) decltype_auto_ret(T x) { return 0; }
+} // namespace ignore
+
+int main() {
+  auto v1 = ignore::auto_ret(5);
+  auto v2 = ignore::with_tag();
+  auto v3 = ignore::decltype_auto_ret(5);
+  auto v4 = ignore::with_tag_template();
+}
Index: lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
===
--- /dev/null
+++ lldb/test/API/functionalities/step-avoids-regexp/TestStepAvoidsRegexp.py
@@ -0,0 +1,47 @@
+"""
+Test thread step-in ignores frames according to the "Avoid regexp" option.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class StepAvoidsRegexTestCase(TestBase):
+def hit_correct_function(self, 

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added inline comments.



Comment at: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py:1
+"""
+Test that we return only the requested template instantiation.

Michael137 wrote:
> I think it would be good to test this with 
> `-gsimple-template-names`/`-gno-simple-template-names`
> 
> You can pass in a dictionary into `self.build()` and override Makefile flags
> 
> E..g, in `lldb/test/API/lang/c/forward/TestForwardDeclaration.py` we do this 
> for `-gdwarf-5`.
> 
> So you could have two `test_XXX` methods like:
> ```
> def do_test(self, debug_flags):
> self.build(dictionary=debug_flags)
> ... actual test ...
> 
> def test_simple_template_name(self):
> do_test(self, dict(CFLAGS_EXTRAS="-gsimple-template-names")
> 
> def test_no_simple_template_name(self):
> do_test(self, dict(CFLAGS_EXTRAS="-gno-simple-template-names")
> ```
thanks, that's very useful to know! done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks updated this revision to Diff 465926.
aeubanks added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
  lldb/test/API/lang/cpp/unique-types2/Makefile
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -0,0 +1,22 @@
+template  class Foo {
+  T t;
+};
+
+template  class FooPack {
+  T t;
+};
+
+int main() {
+  Foo t1;
+  Foo t2;
+  Foo> t3;
+
+  FooPack p1;
+  FooPack p2;
+  FooPack> p3;
+  FooPack t4;
+  FooPack t5;
+  FooPack t6;
+  FooPack t7;
+  // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
@@ -0,0 +1,40 @@
+"""
+Test that we return only the requested template instantiation.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class UniqueTypesTestCase(TestBase):
+def do_test(self, debug_flags):
+"""Test that we only display the requested Foo instantiation, not all Foo instantiations."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("main.cpp"))
+
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'Foo'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack >'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'FooPack'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_name(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_name(self):
+self.do_test(dict(CFLAGS_EXTRAS="-gno-simple-template-names"))
+
Index: lldb/test/API/lang/cpp/unique-types2/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types2/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
===
--- lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
+++ lldb/test/API/lang/cpp/complete-type-check/TestCppIsTypeComplete.py
@@ -51,7 +51,7 @@
 # Record types without a defining declaration are not complete.
 self.assertPointeeIncomplete("FwdClass *", "fwd_class")
 self.assertPointeeIncomplete("FwdClassTypedef *", "fwd_class_typedef")
-self.assertPointeeIncomplete("FwdTemplateClass<> *", "fwd_template_class")
+self.assertPointeeIncomplete("FwdTemplateClass *", "fwd_template_class")
 
 # A pointer type is complete even when it points to an incomplete type.
  

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Qiongsi Wu via Phabricator via lldb-commits]

qiongsiwu1 updated this revision to Diff 465910.
qiongsiwu1 added a comment.

Thanks for the review! Comment addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135400

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options-aranges.c


Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 
2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: "-plugin-opt=-generate-arange-section"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: "-plugin-opt=-generate-arange-section"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Fangrui Song via Phabricator via lldb-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/test/Driver/debug-options-aranges.c:6
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: -plugin-opt=-generate-arange-section




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

https://reviews.llvm.org/D135400

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

[Lldb-commits] [lldb] f491b89 - Revert "Remove the dependency between lib/DebugInfoDWARF and MC."

[Shubham Sandeep Rastogi via lldb-commits]

Author: Shubham Sandeep Rastogi
Date: 2022-10-06T14:58:34-07:00
New Revision: f491b898c5d1312112d14f02d9cbdea7a4b4dc9d

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

LOG: Revert "Remove the dependency between lib/DebugInfoDWARF and MC."

This reverts commit d96ade00c3c96bd451c60e34a17e613cdd5fdc38.

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Symbol/UnwindPlan.cpp
lldb/unittests/Symbol/PostfixExpressionTest.cpp
lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
llvm/include/llvm/DebugInfo/DIContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
llvm/lib/DebugInfo/DWARF/CMakeLists.txt
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
llvm/tools/llvm-objdump/SourcePrinter.cpp
llvm/tools/llvm-objdump/SourcePrinter.h
llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 5284d946dac84..1ccda944cd013 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -69,21 +69,9 @@ void DWARFExpression::UpdateValue(uint64_t const_value,
 
 void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
ABI *abi) const {
-  std::function GetRegName;
-  if (abi) {
-auto RegInfo = abi->GetMCRegisterInfo();
-GetRegName = [RegInfo](uint64_t DwarfRegNum, bool IsEH) -> llvm::StringRef 
{
-  if (llvm::Optional LLVMRegNum =
-  RegInfo.getLLVMRegNum(DwarfRegNum, IsEH))
-if (const char *RegName = RegInfo.getName(*LLVMRegNum))
-  return llvm::StringRef(RegName);
-  return {};
-};
-  }
-  auto DumpOpts = llvm::DIDumpOptions();
-  DumpOpts.GetNameForDWARFReg = GetRegName;
   llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
-  .print(s->AsRawOstream(), DumpOpts, nullptr);
+  .print(s->AsRawOstream(), llvm::DIDumpOptions(),
+ abi ? >GetMCRegisterInfo() : nullptr, nullptr);
 }
 
 RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }

diff  --git a/lldb/source/Symbol/UnwindPlan.cpp 
b/lldb/source/Symbol/UnwindPlan.cpp
index 8ca03344d43a4..bd4690547c6bd 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -84,7 +84,7 @@ static void DumpDWARFExpr(Stream , llvm::ArrayRef 
expr, Thread *threa
 llvm::DataExtractor data(expr, order_and_width->first == eByteOrderLittle,
  order_and_width->second);
 llvm::DWARFExpression(data, order_and_width->second, llvm::dwarf::DWARF32)
-.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr);
+.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr, nullptr);
   } else
 s.PutCString("dwarf-expr");
 }

diff  --git a/lldb/unittests/Symbol/PostfixExpressionTest.cpp 
b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
index 0eaa9d829a656..4d4706d60602d 100644
--- a/lldb/unittests/Symbol/PostfixExpressionTest.cpp
+++ b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
@@ -159,7 +159,7 @@ static std::string ParseAndGenerateDWARF(llvm::StringRef 
expr) {
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, addr_size, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr, nullptr);
   return std::move(os.str());
 }
 

diff  --git 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
index 270cc8e56b7a0..730afb3fbedee 100644
--- 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -40,7 +40,7 @@ CheckValidProgramTranslation(llvm::StringRef fpo_program,
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr, 

[Lldb-commits] [lldb] d96ade0 - Remove the dependency between lib/DebugInfoDWARF and MC.

[Shubham Sandeep Rastogi via lldb-commits]

Author: Shubham Sandeep Rastogi
Date: 2022-10-06T14:46:01-07:00
New Revision: d96ade00c3c96bd451c60e34a17e613cdd5fdc38

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

LOG: Remove the dependency between lib/DebugInfoDWARF and MC.

This patch had to be reverted because on gcc 7.5.0 we see an error converting 
from std::unique_ptr to 
Expected> as the return type for the function 
createRegInfo. This has now been fixed.

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Symbol/UnwindPlan.cpp
lldb/unittests/Symbol/PostfixExpressionTest.cpp
lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
llvm/include/llvm/DebugInfo/DIContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
llvm/lib/DebugInfo/DWARF/CMakeLists.txt
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
llvm/tools/llvm-objdump/SourcePrinter.cpp
llvm/tools/llvm-objdump/SourcePrinter.h
llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 1ccda944cd013..5284d946dac84 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -69,9 +69,21 @@ void DWARFExpression::UpdateValue(uint64_t const_value,
 
 void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
ABI *abi) const {
+  std::function GetRegName;
+  if (abi) {
+auto RegInfo = abi->GetMCRegisterInfo();
+GetRegName = [RegInfo](uint64_t DwarfRegNum, bool IsEH) -> llvm::StringRef 
{
+  if (llvm::Optional LLVMRegNum =
+  RegInfo.getLLVMRegNum(DwarfRegNum, IsEH))
+if (const char *RegName = RegInfo.getName(*LLVMRegNum))
+  return llvm::StringRef(RegName);
+  return {};
+};
+  }
+  auto DumpOpts = llvm::DIDumpOptions();
+  DumpOpts.GetNameForDWARFReg = GetRegName;
   llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
-  .print(s->AsRawOstream(), llvm::DIDumpOptions(),
- abi ? >GetMCRegisterInfo() : nullptr, nullptr);
+  .print(s->AsRawOstream(), DumpOpts, nullptr);
 }
 
 RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }

diff  --git a/lldb/source/Symbol/UnwindPlan.cpp 
b/lldb/source/Symbol/UnwindPlan.cpp
index bd4690547c6bd..8ca03344d43a4 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -84,7 +84,7 @@ static void DumpDWARFExpr(Stream , llvm::ArrayRef 
expr, Thread *threa
 llvm::DataExtractor data(expr, order_and_width->first == eByteOrderLittle,
  order_and_width->second);
 llvm::DWARFExpression(data, order_and_width->second, llvm::dwarf::DWARF32)
-.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr, nullptr);
+.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr);
   } else
 s.PutCString("dwarf-expr");
 }

diff  --git a/lldb/unittests/Symbol/PostfixExpressionTest.cpp 
b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
index 4d4706d60602d..0eaa9d829a656 100644
--- a/lldb/unittests/Symbol/PostfixExpressionTest.cpp
+++ b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
@@ -159,7 +159,7 @@ static std::string ParseAndGenerateDWARF(llvm::StringRef 
expr) {
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, addr_size, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr, nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr);
   return std::move(os.str());
 }
 

diff  --git 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
index 730afb3fbedee..270cc8e56b7a0 100644
--- 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -40,7 +40,7 @@ CheckValidProgramTranslation(llvm::StringRef fpo_program,
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, /*AddressSize=*/4, 

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Qiongsi Wu via Phabricator via lldb-commits]

qiongsiwu1 added a comment.

In D135400#3841321 , @MaskRay wrote:

> I missed https://reviews.llvm.org/D134668 . See my comment there I don't 
> think the change is necessary.

Thanks for the comment! I posted a reply there.


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

https://reviews.llvm.org/D135400

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

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

I missed https://reviews.llvm.org/D134668 . See my comment there I don't think 
the change is necessary.


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

https://reviews.llvm.org/D135400

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

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Qiongsi Wu via Phabricator via lldb-commits]

qiongsiwu1 updated this revision to Diff 465872.
qiongsiwu1 added a comment.

Fix a patch upload error.


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

https://reviews.llvm.org/D135400

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options-aranges.c


Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 
2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: -plugin-opt=-generate-arange-section
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: -plugin-opt=-generate-arange-section
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135400: [clang][LTO] Remove the use of `--` for arange option

[Qiongsi Wu via Phabricator via lldb-commits]

qiongsiwu1 created this revision.
qiongsiwu1 added reviewers: w2yehia, MaskRay, azat, dblaikie.
qiongsiwu1 added projects: clang, LLVM.
Herald added subscribers: ormris, StephenFan, inglorion.
Herald added a project: All.
qiongsiwu1 requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits.
Herald added a project: LLDB.

https://reviews.llvm.org/D134668 attempted to remove all `--` (double dashes) 
when using `plugin-opt` to pass linker options and replaced them with `-`. 
https://reviews.llvm.org/D133092 was committed later but introduced an instance 
of `--`. This patch replaces the `--` with `-`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135400

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options-aranges.c
  lldb/cmake/modules/AddLLDB.cmake


Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -105,7 +105,7 @@
   # this may result in the wrong install DESTINATION. The FRAMEWORK property
   # must be set earlier.
   if(PARAM_FRAMEWORK)
-set_target_properties(liblldb PROPERTIES FRAMEWORK ON)
+set_target_properties(${name} PROPERTIES FRAMEWORK ON)
   endif()
 
   if(PARAM_SHARED)
Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 
2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: -plugin-opt=-generate-arange-section
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: lldb/cmake/modules/AddLLDB.cmake
===
--- lldb/cmake/modules/AddLLDB.cmake
+++ lldb/cmake/modules/AddLLDB.cmake
@@ -105,7 +105,7 @@
   # this may result in the wrong install DESTINATION. The FRAMEWORK property
   # must be set earlier.
   if(PARAM_FRAMEWORK)
-set_target_properties(liblldb PROPERTIES FRAMEWORK ON)
+set_target_properties(${name} PROPERTIES FRAMEWORK ON)
   endif()
 
   if(PARAM_SHARED)
Index: clang/test/Driver/debug-options-aranges.c
===
--- clang/test/Driver/debug-options-aranges.c
+++ clang/test/Driver/debug-options-aranges.c
@@ -3,4 +3,4 @@
 /// Check that the linker plugin will get -generate-arange-section.
 // RUN: %clang -### -g --target=x86_64-linux -flto  -gdwarf-aranges %s 2>&1 | FileCheck %s
 // RUN: %clang -### -g --target=x86_64-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck %s
-// CHECK: --plugin-opt=-generate-arange-section
+// CHECK: -plugin-opt=-generate-arange-section
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -514,7 +514,7 @@
   // the way out.
   if (Args.hasArg(options::OPT_gdwarf_aranges)) {
 CmdArgs.push_back(
-Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+Args.MakeArgString("-plugin-opt=-generate-arange-section"));
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D135399: [NFCI] More TypeCategoryImpl refactoring.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added reviewers: labath, jingham.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

The main aim of this patch is to delete the remaining instances of code
reaching into the internals of `TypeCategoryImpl`. I made the following
changes:

- Add some more methods to `TieredFormatterContainer` and `TypeCategoryImpl` to 
expose functionality that is implemented in `FormattersContainer`.

- Add new overloads of `TypeCategoryImpl::AddTypeXXX` to make it easier to add 
formatters to categories without reaching into the internal 
`FormattersContainer` objects.

- Remove the `GetTypeXXXContainer` and `GetRegexTypeXXXContainer` accessors 
from `TypeCategoryImpl` and update all call sites to use the new methods 
instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135399

Files:
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/DataFormatters/FormatManager.cpp
  lldb/source/DataFormatters/FormattersHelpers.cpp
  lldb/source/DataFormatters/TypeCategory.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

Index: lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -284,12 +284,12 @@
 
   lldb::TypeSummaryImplSP ObjC_BOOL_summary(new CXXFunctionSummaryFormat(
   objc_flags, lldb_private::formatters::ObjCBOOLSummaryProvider, ""));
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL"),
- ObjC_BOOL_summary);
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL &"),
- ObjC_BOOL_summary);
-  objc_category_sp->GetTypeSummariesContainer()->Add(ConstString("BOOL *"),
- ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL", eFormatterMatchExact,
+   ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL &", eFormatterMatchExact,
+   ObjC_BOOL_summary);
+  objc_category_sp->AddTypeSummary("BOOL *", eFormatterMatchExact,
+   ObjC_BOOL_summary);
 
   // we need to skip pointers here since we are special casing a SEL* when
   // retrieving its value
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -764,8 +764,8 @@
   ConstString("^std::__[[:alnum:]]+::span<.+>(( )?&)?$"), stl_deref_flags,
   true);
 
-  cpp_category_sp->GetRegexTypeSyntheticsContainer()->Add(
-  RegularExpression("^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$"),
+  cpp_category_sp->AddTypeSynthetic(
+  "^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$", eFormatterMatchRegex,
   SyntheticChildrenSP(new ScriptedSyntheticChildren(
   stl_synth_flags,
   "lldb.formatters.cpp.libcxx.stddeque_SynthProvider")));
@@ -958,55 +958,52 @@
   stl_summary_flags, LibStdcppWStringSummaryProvider,
   "libstdc++ c++11 std::wstring summary provider"));
 
-  cpp_category_sp->GetTypeSummariesContainer()->Add(ConstString("std::string"),
-std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string"), std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string,std::"
-  "allocator >"),
-  std_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::basic_string, "
-  "std::allocator >"),
-  std_string_summary_sp);
-
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::string"), cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
-  cpp_category_sp->GetTypeSummariesContainer()->Add(
-  ConstString("std::__cxx11::basic_string, "
-  "std::allocator >"),
-  cxx11_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::string", eFormatterMatchExact,
+  std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary("std::basic_string",
+  eFormatterMatchRegex, std_string_summary_sp);
+  cpp_category_sp->AddTypeSummary(
+  "std::basic_string,std::allocator >",
+  

[Lldb-commits] [lldb] 69c661a - [lldb] Skip check for conflicting filter/synth when adding a new regex.

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2022-10-06T13:27:11-07:00
New Revision: 69c661a65ff8c04ce9408a788a77df67481545a1

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

LOG: [lldb] Skip check for conflicting filter/synth when adding a new regex.

When adding a new synthetic child provider, we check for an existing
conflicting filter in the same category (and vice versa). This is done
by trying to match the new type name against registered formatters.

However, the new type name we're registered can also be a regex
(`type synth add -x`), and in this case the conflict check is just
wrong: it will try to match the new regex as if it was a type name,
against previously registered regexes.

See https://github.com/llvm/llvm-project/issues/57947 for a longer
explanation with concrete examples of incorrect behavior.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectType.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectType.cpp 
b/lldb/source/Commands/CommandObjectType.cpp
index b78f98ce132b..01b3ec2fed14 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -2319,12 +2319,17 @@ bool CommandObjectTypeSynthAdd::AddSynth(ConstString 
type_name,
   type = eRegexSynth;
   }
 
-  if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
-if (error)
-  error->SetErrorStringWithFormat("cannot add synthetic for type %s when "
-  "filter is defined in same category!",
-  type_name.AsCString());
-return false;
+  // Only check for conflicting filters in the same category if `type_name` is
+  // an actual type name. Matching a regex string against registered regexes
+  // doesn't work.
+  if (type == eRegularSynth) {
+if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
+  if (error)
+error->SetErrorStringWithFormat("cannot add synthetic for type %s when 
"
+"filter is defined in same category!",
+type_name.AsCString());
+  return false;
+}
   }
 
   if (type == eRegexSynth) {
@@ -2442,13 +2447,18 @@ class CommandObjectTypeFilterAdd : public 
CommandObjectParsed {
 type = eRegexFilter;
 }
 
-if (category->AnyMatches(type_name, eFormatCategoryItemSynth, false)) {
-  if (error)
-error->SetErrorStringWithFormat("cannot add filter for type %s when "
-"synthetic is defined in same "
-"category!",
-type_name.AsCString());
-  return false;
+// Only check for conflicting synthetic child providers in the same 
category
+// if `type_name` is an actual type name. Matching a regex string against
+// registered regexes doesn't work.
+if (type == eRegularFilter) {
+  if (category->AnyMatches(type_name, eFormatCategoryItemSynth, false)) {
+if (error)
+  error->SetErrorStringWithFormat("cannot add filter for type %s when "
+  "synthetic is defined in same "
+  "category!",
+  type_name.AsCString());
+return false;
+  }
 }
 
 if (type == eRegexFilter) {

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
index 40a26623bccf..7eef1bc990cc 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
@@ -257,6 +257,22 @@ def cleanup():
 self.expect("frame variable f00_1", matching=False,
 substrs=['fake_a = '])
 
+# check that we don't feed a regex into another regex when checking for
+# existing conflicting synth/filters. The two following expressions
+# accept 
diff erent types: one will accept types that look like an array
+# of MyType, the other will accept types that contain "MyType1" or
+# "MyType2". But the second regex looks like an array of MyType, so
+# lldb used to incorrectly reject it.
+self.runCmd(r'type synth add -l fooSynthProvider -x 
"^MyType\[[0-9]+]$"')
+self.runCmd(r'type filter add 

[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG69c661a65ff8: [lldb] Skip check for conflicting filter/synth 
when adding a new regex. (authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134570

Files:
  lldb/source/Commands/CommandObjectType.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py


Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
@@ -257,6 +257,22 @@
 self.expect("frame variable f00_1", matching=False,
 substrs=['fake_a = '])
 
+# check that we don't feed a regex into another regex when checking for
+# existing conflicting synth/filters. The two following expressions
+# accept different types: one will accept types that look like an array
+# of MyType, the other will accept types that contain "MyType1" or
+# "MyType2". But the second regex looks like an array of MyType, so
+# lldb used to incorrectly reject it.
+self.runCmd(r'type synth add -l fooSynthProvider -x 
"^MyType\[[0-9]+]$"')
+self.runCmd(r'type filter add --child a -x "MyType[12]"')
+
+# Same, but adding the filter first to verify the check when doing
+# `type synth add`. We need to delete the synth from the previous test
+# first.
+self.runCmd(r'type synth delete "^MyType\[[0-9]+]$"')
+self.runCmd(r'type filter add --child a -x "^MyType\[[0-9]+]$"')
+self.runCmd(r'type synth add -l fooSynthProvider -x "MyType[12]"')
+
 def rdar10960550_formatter_commands(self):
 """Test that synthetic children persist stoppoints."""
 self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2319,12 +2319,17 @@
   type = eRegexSynth;
   }
 
-  if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
-if (error)
-  error->SetErrorStringWithFormat("cannot add synthetic for type %s when "
-  "filter is defined in same category!",
-  type_name.AsCString());
-return false;
+  // Only check for conflicting filters in the same category if `type_name` is
+  // an actual type name. Matching a regex string against registered regexes
+  // doesn't work.
+  if (type == eRegularSynth) {
+if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
+  if (error)
+error->SetErrorStringWithFormat("cannot add synthetic for type %s when 
"
+"filter is defined in same category!",
+type_name.AsCString());
+  return false;
+}
   }
 
   if (type == eRegexSynth) {
@@ -2442,13 +2447,18 @@
 type = eRegexFilter;
 }
 
-if (category->AnyMatches(type_name, eFormatCategoryItemSynth, false)) {
-  if (error)
-error->SetErrorStringWithFormat("cannot add filter for type %s when "
-"synthetic is defined in same "
-"category!",
-type_name.AsCString());
-  return false;
+// Only check for conflicting synthetic child providers in the same 
category
+// if `type_name` is an actual type name. Matching a regex string against
+// registered regexes doesn't work.
+if (type == eRegularFilter) {
+  if (category->AnyMatches(type_name, eFormatCategoryItemSynth, false)) {
+if (error)
+  error->SetErrorStringWithFormat("cannot add filter for type %s when "
+  "synthetic is defined in same "
+  "category!",
+  type_name.AsCString());
+return false;
+  }
 }
 
 if (type == eRegexFilter) {


Index: lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
@@ -257,6 +257,22 @@
 

[Lldb-commits] [PATCH] D134771: [NFCI] Simplify TypeCategoryImpl for-each callbacks.

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdee9c7f5d7e5: [NFCI] Simplify TypeCategoryImpl for-each 
callbacks. (authored by jgorbe).

Changed prior to commit:
  https://reviews.llvm.org/D134771?vs=463624=465824#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134771

Files:
  lldb/include/lldb/DataFormatters/TypeCategory.h
  lldb/source/Commands/CommandObjectType.cpp

Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -1097,36 +1097,20 @@
   "---\nCategory: %s%s\n---\n",
   category->GetName(), category->IsEnabled() ? "" : " (disabled)");
 
-  TypeCategoryImpl::ForEachCallbacks foreach;
-  foreach
-.SetExact([, _regex, _printed](
-  const TypeMatcher _matcher,
-  const FormatterSharedPointer _sp) -> bool {
-  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
- formatter_regex.get())) {
-any_printed = true;
-result.GetOutputStream().Printf(
-"%s: %s\n", type_matcher.GetMatchString().GetCString(),
-format_sp->GetDescription().c_str());
-  }
-  return true;
-});
-
-  foreach
-.SetWithRegex([, _regex, _printed](
-  const TypeMatcher _matcher,
-  const FormatterSharedPointer _sp) -> bool {
-  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
- formatter_regex.get())) {
-any_printed = true;
-result.GetOutputStream().Printf(
-"%s: %s\n", type_matcher.GetMatchString().GetCString(),
-format_sp->GetDescription().c_str());
-  }
-  return true;
-});
-
-  category->ForEach(foreach);
+  TypeCategoryImpl::ForEachCallback print_formatter =
+  [, _regex,
+   _printed](const TypeMatcher _matcher,
+ const FormatterSharedPointer _sp) -> bool {
+if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+   formatter_regex.get())) {
+  any_printed = true;
+  result.GetOutputStream().Printf(
+  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
+  format_sp->GetDescription().c_str());
+}
+return true;
+  };
+  category->ForEach(print_formatter);
 };
 
 if (m_options.m_category_language.OptionWasSet()) {
Index: lldb/include/lldb/DataFormatters/TypeCategory.h
===
--- lldb/include/lldb/DataFormatters/TypeCategory.h
+++ lldb/include/lldb/DataFormatters/TypeCategory.h
@@ -130,6 +130,16 @@
 return lldb::TypeNameSpecifierImplSP();
   }
 
+  /// Iterates through tiers in order, running `callback` on each element of
+  /// each tier.
+  void ForEach(std::function &)>
+   callback) {
+for (auto sc : m_subcontainers) {
+  sc->ForEach(callback);
+}
+  }
+
  private:
   std::array, lldb::eLastFormatterMatchType + 1>
   m_subcontainers;
@@ -146,120 +156,36 @@
   typedef uint16_t FormatCategoryItems;
   static const uint16_t ALL_ITEM_TYPES = UINT16_MAX;
 
-  template  class ForEachCallbacks {
-  public:
-ForEachCallbacks() = default;
-~ForEachCallbacks() = default;
-
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetExact(FormatContainer::ForEachCallback callback) {
-  m_format_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetWithRegex(FormatContainer::ForEachCallback callback) {
-  m_format_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetExact(SummaryContainer::ForEachCallback callback) {
-  m_summary_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetWithRegex(SummaryContainer::ForEachCallback callback) {
-  m_summary_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetExact(FilterContainer::ForEachCallback callback) {
-  m_filter_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks &>::type
-SetWithRegex(FilterContainer::ForEachCallback callback) {
-  m_filter_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename 

[Lldb-commits] [lldb] dee9c7f - [NFCI] Simplify TypeCategoryImpl for-each callbacks.

[Jorge Gorbe Moya via lldb-commits]

Author: Jorge Gorbe Moya
Date: 2022-10-06T12:11:27-07:00
New Revision: dee9c7f5d7e53aa6a9e7f85cdf9935341ba7e636

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

LOG: [NFCI] Simplify TypeCategoryImpl for-each callbacks.

The callback system to iterate over every formatter of a given kind in
a TypeCategoryImpl is only used in one place (the implementation of
`type {formatter_kind} list`), and it's too convoluted for the sake of
unused flexibility.

This change changes it so that only one callback is passed to `ForEach`
(instead of a callback for exact matches and another one for regex
matches), and moves the iteration logic to `TieredFormatterContainer`
to avoid duplication.

If in the future we need different logic in the callback depending on
exact/regex match, the callback can get the type of formatter matching
used from the TypeMatcher argument anyway.

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

Added: 


Modified: 
lldb/include/lldb/DataFormatters/TypeCategory.h
lldb/source/Commands/CommandObjectType.cpp

Removed: 




diff  --git a/lldb/include/lldb/DataFormatters/TypeCategory.h 
b/lldb/include/lldb/DataFormatters/TypeCategory.h
index 2f0cb6c78472f..edf3f5d6605e4 100644
--- a/lldb/include/lldb/DataFormatters/TypeCategory.h
+++ b/lldb/include/lldb/DataFormatters/TypeCategory.h
@@ -130,6 +130,16 @@ template  class 
TieredFormatterContainer {
 return lldb::TypeNameSpecifierImplSP();
   }
 
+  /// Iterates through tiers in order, running `callback` on each element of
+  /// each tier.
+  void ForEach(std::function &)>
+   callback) {
+for (auto sc : m_subcontainers) {
+  sc->ForEach(callback);
+}
+  }
+
  private:
   std::array, lldb::eLastFormatterMatchType + 1>
   m_subcontainers;
@@ -146,120 +156,36 @@ class TypeCategoryImpl {
   typedef uint16_t FormatCategoryItems;
   static const uint16_t ALL_ITEM_TYPES = UINT16_MAX;
 
-  template  class ForEachCallbacks {
-  public:
-ForEachCallbacks() = default;
-~ForEachCallbacks() = default;
-
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetExact(FormatContainer::ForEachCallback callback) {
-  m_format_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetWithRegex(FormatContainer::ForEachCallback callback) {
-  m_format_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetExact(SummaryContainer::ForEachCallback callback) {
-  m_summary_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetWithRegex(SummaryContainer::ForEachCallback callback) {
-  m_summary_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetExact(FilterContainer::ForEachCallback callback) {
-  m_filter_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetWithRegex(FilterContainer::ForEachCallback callback) {
-  m_filter_regex = std::move(callback);
-  return *this;
-}
-
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetExact(SynthContainer::ForEachCallback callback) {
-  m_synth_exact = std::move(callback);
-  return *this;
-}
-template 
-typename std::enable_if::value, ForEachCallbacks 
&>::type
-SetWithRegex(SynthContainer::ForEachCallback callback) {
-  m_synth_regex = std::move(callback);
-  return *this;
-}
-
-FormatContainer::ForEachCallback GetFormatExactCallback() const {
-  return m_format_exact;
-}
-FormatContainer::ForEachCallback GetFormatRegexCallback() const {
-  return m_format_regex;
-}
-
-SummaryContainer::ForEachCallback GetSummaryExactCallback() const {
-  return m_summary_exact;
-}
-SummaryContainer::ForEachCallback GetSummaryRegexCallback() const {
-  return m_summary_regex;
-}
-
-FilterContainer::ForEachCallback GetFilterExactCallback() const {
-  return m_filter_exact;
-}
-FilterContainer::ForEachCallback GetFilterRegexCallback() const {
-  return m_filter_regex;
-}
-
-SynthContainer::ForEachCallback GetSynthExactCallback() const {
-  return m_synth_exact;
-}
-SynthContainer::ForEachCallback GetSynthRegexCallback() const {
-  return m_synth_regex;
-}
-
-  private:
-FormatContainer::ForEachCallback m_format_exact;
-FormatContainer::ForEachCallback m_format_regex;
-
-

[Lldb-commits] [lldb] 01470b6 - [lldb] Fix hard-coded argument to set_target_properties

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-10-06T11:43:52-07:00
New Revision: 01470b68f392af8bbf95b2ab48253b1662e2cdc7

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

LOG: [lldb] Fix hard-coded argument to set_target_properties

The call to `set_target_properties` should use the target passed to
`add_lldb_library` instead of a hard-coded value. Upstream `liblldb` is
the only target for which this matters, but downstream we have
LLDBRPC.framework which needs this as well.

Added: 


Modified: 
lldb/cmake/modules/AddLLDB.cmake

Removed: 




diff  --git a/lldb/cmake/modules/AddLLDB.cmake 
b/lldb/cmake/modules/AddLLDB.cmake
index 3291a7c808e16..ea52b47d6d727 100644
--- a/lldb/cmake/modules/AddLLDB.cmake
+++ b/lldb/cmake/modules/AddLLDB.cmake
@@ -105,7 +105,7 @@ function(add_lldb_library name)
   # this may result in the wrong install DESTINATION. The FRAMEWORK property
   # must be set earlier.
   if(PARAM_FRAMEWORK)
-set_target_properties(liblldb PROPERTIES FRAMEWORK ON)
+set_target_properties(${name} PROPERTIES FRAMEWORK ON)
   endif()
 
   if(PARAM_SHARED)



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

[Lldb-commits] [lldb] 870b74d - Revert "Remove the dependency between lib/DebugInfoDWARF and MC."

[Shubham Sandeep Rastogi via lldb-commits]

Author: Shubham Sandeep Rastogi
Date: 2022-10-06T09:30:46-07:00
New Revision: 870b74d59052143c6e3a3e70d6e5c17cd43ff58f

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

LOG: Revert "Remove the dependency between lib/DebugInfoDWARF and MC."

This reverts commit 0008990479a2daf587c2a4f274384b2fb87247fb.

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Symbol/UnwindPlan.cpp
lldb/unittests/Symbol/PostfixExpressionTest.cpp
lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
llvm/include/llvm/DebugInfo/DIContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
llvm/lib/DebugInfo/DWARF/CMakeLists.txt
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
llvm/tools/llvm-objdump/SourcePrinter.cpp
llvm/tools/llvm-objdump/SourcePrinter.h
llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 5284d946dac84..1ccda944cd013 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -69,21 +69,9 @@ void DWARFExpression::UpdateValue(uint64_t const_value,
 
 void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
ABI *abi) const {
-  std::function GetRegName;
-  if (abi) {
-auto RegInfo = abi->GetMCRegisterInfo();
-GetRegName = [RegInfo](uint64_t DwarfRegNum, bool IsEH) -> llvm::StringRef 
{
-  if (llvm::Optional LLVMRegNum =
-  RegInfo.getLLVMRegNum(DwarfRegNum, IsEH))
-if (const char *RegName = RegInfo.getName(*LLVMRegNum))
-  return llvm::StringRef(RegName);
-  return {};
-};
-  }
-  auto DumpOpts = llvm::DIDumpOptions();
-  DumpOpts.GetNameForDWARFReg = GetRegName;
   llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
-  .print(s->AsRawOstream(), DumpOpts, nullptr);
+  .print(s->AsRawOstream(), llvm::DIDumpOptions(),
+ abi ? >GetMCRegisterInfo() : nullptr, nullptr);
 }
 
 RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }

diff  --git a/lldb/source/Symbol/UnwindPlan.cpp 
b/lldb/source/Symbol/UnwindPlan.cpp
index 8ca03344d43a4..bd4690547c6bd 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -84,7 +84,7 @@ static void DumpDWARFExpr(Stream , llvm::ArrayRef 
expr, Thread *threa
 llvm::DataExtractor data(expr, order_and_width->first == eByteOrderLittle,
  order_and_width->second);
 llvm::DWARFExpression(data, order_and_width->second, llvm::dwarf::DWARF32)
-.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr);
+.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr, nullptr);
   } else
 s.PutCString("dwarf-expr");
 }

diff  --git a/lldb/unittests/Symbol/PostfixExpressionTest.cpp 
b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
index 0eaa9d829a656..4d4706d60602d 100644
--- a/lldb/unittests/Symbol/PostfixExpressionTest.cpp
+++ b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
@@ -159,7 +159,7 @@ static std::string ParseAndGenerateDWARF(llvm::StringRef 
expr) {
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, addr_size, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr, nullptr);
   return std::move(os.str());
 }
 

diff  --git 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
index 270cc8e56b7a0..730afb3fbedee 100644
--- 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -40,7 +40,7 @@ CheckValidProgramTranslation(llvm::StringRef fpo_program,
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr, 

[Lldb-commits] [PATCH] D134817: Remove the dependency between lib/DebugInfoDWARF and MC

[Shubham Sandeep Rastogi 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 rG0008990479a2: Remove the dependency between 
lib/DebugInfoDWARF and MC. (authored by rastogishubham).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134817

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/source/Symbol/UnwindPlan.cpp
  lldb/unittests/Symbol/PostfixExpressionTest.cpp
  lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
  llvm/include/llvm/DebugInfo/DIContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
  llvm/lib/DebugInfo/DWARF/CMakeLists.txt
  llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
  llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
  llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
  llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/tools/llvm-objdump/SourcePrinter.cpp
  llvm/tools/llvm-objdump/SourcePrinter.h
  llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
  llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
  llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp

Index: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp
@@ -60,7 +60,17 @@
   raw_string_ostream OS(Result);
   DataExtractor DE(ExprData, true, 8);
   DWARFExpression Expr(DE, 8);
-  Expr.printCompact(OS, *MRI);
+
+  auto GetRegName = [&](uint64_t DwarfRegNum, bool IsEH) -> StringRef {
+if (llvm::Optional LLVMRegNum =
+this->MRI->getLLVMRegNum(DwarfRegNum, IsEH))
+  if (const char *RegName = this->MRI->getName(*LLVMRegNum))
+return llvm::StringRef(RegName);
+OS << "";
+return {};
+  };
+
+  Expr.printCompact(OS, GetRegName);
   EXPECT_EQ(OS.str(), Expected);
 }
 
Index: llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
===
--- llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
+++ llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
@@ -41,7 +41,9 @@
   StringRef ExpectedFirstLine) {
   std::string Output;
   raw_string_ostream OS(Output);
-  TestCIE.dump(OS, DIDumpOptions(), /*MRI=*/nullptr, IsEH);
+  auto DumpOpts = DIDumpOptions();
+  DumpOpts.IsEH = IsEH;
+  TestCIE.dump(OS, DumpOpts);
   OS.flush();
   StringRef FirstLine = StringRef(Output).split('\n').first;
   EXPECT_EQ(FirstLine, ExpectedFirstLine);
@@ -51,7 +53,9 @@
   StringRef ExpectedFirstLine) {
   std::string Output;
   raw_string_ostream OS(Output);
-  TestFDE.dump(OS, DIDumpOptions(), /*MRI=*/nullptr, IsEH);
+  auto DumpOpts = DIDumpOptions();
+  DumpOpts.IsEH = IsEH;
+  TestFDE.dump(OS, DumpOpts);
   OS.flush();
   StringRef FirstLine = StringRef(Output).split('\n').first;
   EXPECT_EQ(FirstLine, ExpectedFirstLine);
Index: llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
===
--- llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
+++ llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
@@ -226,8 +226,9 @@
 W.getOStream() << "\n";
 W.startLine() << "Program:\n";
 W.indent();
-Entry.cfis().dump(W.getOStream(), DIDumpOptions(), nullptr,
-  W.getIndentLevel());
+auto DumpOpts = DIDumpOptions();
+DumpOpts.IsEH = true;
+Entry.cfis().dump(W.getOStream(), DumpOpts, W.getIndentLevel());
 W.unindent();
 W.unindent();
 W.getOStream() << "\n";
Index: llvm/tools/llvm-objdump/SourcePrinter.h
===
--- llvm/tools/llvm-objdump/SourcePrinter.h
+++ llvm/tools/llvm-objdump/SourcePrinter.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/Symbolize/Symbolize.h"
+#include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/FormattedStream.h"
 #include 
Index: llvm/tools/llvm-objdump/SourcePrinter.cpp
===
--- llvm/tools/llvm-objdump/SourcePrinter.cpp
+++ llvm/tools/llvm-objdump/SourcePrinter.cpp
@@ -42,7 +42,16 @@
   DataExtractor Data({LocExpr.Expr.data(), LocExpr.Expr.size()},
  Unit->getContext().isLittleEndian(), 0);
   DWARFExpression Expression(Data, Unit->getAddressByteSize());
-  

[Lldb-commits] [lldb] 0008990 - Remove the dependency between lib/DebugInfoDWARF and MC.

[Shubham Sandeep Rastogi via lldb-commits]

Author: Shubham Sandeep Rastogi
Date: 2022-10-06T09:25:57-07:00
New Revision: 0008990479a2daf587c2a4f274384b2fb87247fb

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

LOG: Remove the dependency between lib/DebugInfoDWARF and MC.

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

Added: 


Modified: 
lldb/source/Expression/DWARFExpression.cpp
lldb/source/Symbol/UnwindPlan.cpp
lldb/unittests/Symbol/PostfixExpressionTest.cpp
lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
llvm/include/llvm/DebugInfo/DIContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFContext.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h
llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h
llvm/include/llvm/DebugInfo/DWARF/DWARFExpression.h
llvm/lib/DebugInfo/DWARF/CMakeLists.txt
llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp
llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp
llvm/lib/DebugInfo/DWARF/DWARFDie.cpp
llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp
llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
llvm/tools/llvm-objdump/SourcePrinter.cpp
llvm/tools/llvm-objdump/SourcePrinter.h
llvm/tools/llvm-readobj/DwarfCFIEHPrinter.h
llvm/unittests/DebugInfo/DWARF/DWARFDebugFrameTest.cpp
llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp

Removed: 




diff  --git a/lldb/source/Expression/DWARFExpression.cpp 
b/lldb/source/Expression/DWARFExpression.cpp
index 1ccda944cd013..5284d946dac84 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -69,9 +69,21 @@ void DWARFExpression::UpdateValue(uint64_t const_value,
 
 void DWARFExpression::DumpLocation(Stream *s, lldb::DescriptionLevel level,
ABI *abi) const {
+  std::function GetRegName;
+  if (abi) {
+auto RegInfo = abi->GetMCRegisterInfo();
+GetRegName = [RegInfo](uint64_t DwarfRegNum, bool IsEH) -> llvm::StringRef 
{
+  if (llvm::Optional LLVMRegNum =
+  RegInfo.getLLVMRegNum(DwarfRegNum, IsEH))
+if (const char *RegName = RegInfo.getName(*LLVMRegNum))
+  return llvm::StringRef(RegName);
+  return {};
+};
+  }
+  auto DumpOpts = llvm::DIDumpOptions();
+  DumpOpts.GetNameForDWARFReg = GetRegName;
   llvm::DWARFExpression(m_data.GetAsLLVM(), m_data.GetAddressByteSize())
-  .print(s->AsRawOstream(), llvm::DIDumpOptions(),
- abi ? >GetMCRegisterInfo() : nullptr, nullptr);
+  .print(s->AsRawOstream(), DumpOpts, nullptr);
 }
 
 RegisterKind DWARFExpression::GetRegisterKind() const { return m_reg_kind; }

diff  --git a/lldb/source/Symbol/UnwindPlan.cpp 
b/lldb/source/Symbol/UnwindPlan.cpp
index bd4690547c6bd..8ca03344d43a4 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -84,7 +84,7 @@ static void DumpDWARFExpr(Stream , llvm::ArrayRef 
expr, Thread *threa
 llvm::DataExtractor data(expr, order_and_width->first == eByteOrderLittle,
  order_and_width->second);
 llvm::DWARFExpression(data, order_and_width->second, llvm::dwarf::DWARF32)
-.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr, nullptr);
+.print(s.AsRawOstream(), llvm::DIDumpOptions(), nullptr);
   } else
 s.PutCString("dwarf-expr");
 }

diff  --git a/lldb/unittests/Symbol/PostfixExpressionTest.cpp 
b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
index 4d4706d60602d..0eaa9d829a656 100644
--- a/lldb/unittests/Symbol/PostfixExpressionTest.cpp
+++ b/lldb/unittests/Symbol/PostfixExpressionTest.cpp
@@ -159,7 +159,7 @@ static std::string ParseAndGenerateDWARF(llvm::StringRef 
expr) {
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, addr_size, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr, nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr);
   return std::move(os.str());
 }
 

diff  --git 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
index 730afb3fbedee..270cc8e56b7a0 100644
--- 
a/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
+++ 
b/lldb/unittests/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpressionTests.cpp
@@ -40,7 +40,7 @@ CheckValidProgramTranslation(llvm::StringRef fpo_program,
   std::string result;
   llvm::raw_string_ostream os(result);
   llvm::DWARFExpression(extractor, /*AddressSize=*/4, llvm::dwarf::DWARF32)
-  .print(os, llvm::DIDumpOptions(), nullptr, nullptr);
+  .print(os, llvm::DIDumpOptions(), nullptr);
 
   // 

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CleanupProcess

[Pavel Labath via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG08c4a6795ac4: [lldb] Move breakpoint hit reset code to 
Target::CleanupProcess (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134882

Files:
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,57 @@
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def readRegister(self, reg):
+# Pretend we're at the breakpoint after we've been resumed.
+return "3412" if self.continued else 
"4747"
+
+def cont(self):
+self.continued = True
+return "T05thread=47;reason:breakpoint"
+
+# Connect to the first process and set our breakpoint.
+self.server.responder = MyResponder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+bkpt = target.BreakpointCreateByAddress(0x1234)
+self.assertTrue(bkpt.IsValid())
+self.assertEqual(bkpt.GetNumLocations(), 1)
+
+# "continue" the process. It should hit our breakpoint.
+process.Continue()
+self.assertState(process.GetState(), lldb.eStateStopped)
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Now kill it. The breakpoint should still show a hit count of one.
+process.Kill()
+self.server.stop()
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Start over, and reconnect.
+self.server = MockGDBServer(self.server_socket_class())
+self.server.start()
+
+process = self.connect(target)
+
+# The hit count should be reset.
+self.assertEqual(bkpt.GetHitCount(), 0)
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -177,6 +177,7 @@
   // clean up needs some help from the process.
   m_breakpoint_list.ClearAllBreakpointSites();
   m_internal_breakpoint_list.ClearAllBreakpointSites();
+  ResetBreakpointHitCounts();
   // Disable watchpoints just on the debugger side.
   std::unique_lock lock;
   this->GetWatchpointList().GetListMutex(lock);
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2761,18 +2761,15 @@
 }
 
 Status Process::WillLaunch(Module *module) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillLaunch(module);
 }
 
 Status Process::WillAttachToProcessWithID(lldb::pid_t pid) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillAttachToProcessWithID(pid);
 }
 
 Status Process::WillAttachToProcessWithName(const char *process_name,
 bool wait_for_launch) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillAttachToProcessWithName(process_name, wait_for_launch);
 }
 


Index: lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,57 @@
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+

[Lldb-commits] [PATCH] D134754: [lldb/gdb-server] Better reporting of launch errors

[Pavel Labath via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d1de7b34af4: [lldb/gdb-server] Better reporting of launch 
errors (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134754

Files:
  lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -86,7 +86,30 @@
 error = lldb.SBError()
 target.Launch(lldb.SBListener(), None, None, None, None, None,
 None, 0, True, error)
-self.assertEquals("'A' packet returned an error: 71", error.GetCString())
+self.assertRegex(error.GetCString(), "Cannot launch '.*a': Error 71")
+
+def test_launch_rich_error(self):
+class MyResponder(MockGDBServerResponder):
+def qC(self):
+return "E42"
+
+def qfThreadInfo(self):
+return "OK" # No threads.
+
+# Then, when we are asked to attach, error out.
+def vRun(self, packet):
+return "Eff;" + seven.hexlify("I'm a teapot")
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(self, self.dbg.GetListener(), process, [lldb.eStateConnected])
+
+error = lldb.SBError()
+target.Launch(lldb.SBListener(), None, None, None, None, None,
+None, 0, True, error)
+self.assertRegex(error.GetCString(), "Cannot launch '.*a': I'm a teapot")
 
 def test_read_registers_using_g_packets(self):
 """Test reading registers using 'g' packets (default behavior)"""
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -84,6 +84,7 @@
 
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/raw_ostream.h"
 
@@ -799,17 +800,17 @@
   GDBRemoteCommunication::ScopedTimeout timeout(m_gdb_comm,
 std::chrono::seconds(10));
 
-  int arg_packet_err = m_gdb_comm.SendArgumentsPacket(launch_info);
-  if (arg_packet_err == 0) {
-std::string error_str;
-if (m_gdb_comm.GetLaunchSuccess(error_str)) {
-  SetID(m_gdb_comm.GetCurrentProcessID());
-} else {
-  error.SetErrorString(error_str.c_str());
-}
+  // Since we can't send argv0 separate from the executable path, we need to
+  // make sure to use the actual executable path found in the launch_info...
+  Args args = launch_info.GetArguments();
+  if (FileSpec exe_file = launch_info.GetExecutableFile())
+args.ReplaceArgumentAtIndex(0, exe_file.GetPath(false));
+  if (llvm::Error err = m_gdb_comm.LaunchProcess(args)) {
+error.SetErrorStringWithFormatv("Cannot launch '{0}': {1}",
+args.GetArgumentAtIndex(0),
+llvm::fmt_consume(std::move(err)));
   } else {
-error.SetErrorStringWithFormat("'A' packet returned an error: %i",
-   arg_packet_err);
+SetID(m_gdb_comm.GetCurrentProcessID());
   }
 }
 
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -80,8 +80,6 @@
 
   lldb::pid_t GetCurrentProcessID(bool allow_lazy = true);
 
-  bool GetLaunchSuccess(std::string _str);
-
   bool LaunchGDBServer(const char *remote_accept_hostname, lldb::pid_t ,
uint16_t , std::string _name);
 
@@ -90,19 +88,11 @@
 
   bool KillSpawnedProcess(lldb::pid_t pid);
 
-  /// Sends a GDB remote protocol 'A' packet that delivers program
-  /// arguments to the remote server.
-  ///
-  /// \param[in] launch_info
-  /// A NULL terminated array of const C strings 

[Lldb-commits] [lldb] 8d1de7b - [lldb/gdb-server] Better reporting of launch errors

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-10-06T17:18:51+02:00
New Revision: 8d1de7b34af46a089eb5433c700419ad9b2923ee

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

LOG: [lldb/gdb-server] Better reporting of launch errors

Use our "rich error" facility to propagate error reported by the stub to
the user. lldb-server reports rich launch errors as of D133352.

To make this easier to implement, and reduce code duplication, I have
moved the vRun/A/qLaunchSuccess handling into a single
GDBRemoteCommunicationClient function.

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

Added: 


Modified: 
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp 
b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
index 7f92b669641df..a20643ad21c23 100644
--- a/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ b/lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -29,6 +29,7 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/Utility/UriParser.h"
+#include "llvm/Support/FormatAdapters.h"
 
 #include "Plugins/Process/Utility/GDBRemoteSignals.h"
 #include "Plugins/Process/gdb-remote/ProcessGDBRemote.h"
@@ -361,39 +362,36 @@ Status 
PlatformRemoteGDBServer::LaunchProcess(ProcessLaunchInfo _info) {
   "PlatformRemoteGDBServer::%s() set launch architecture triple to '%s'",
   __FUNCTION__, arch_triple ? arch_triple : "");
 
-  int arg_packet_err;
   {
 // Scope for the scoped timeout object
 process_gdb_remote::GDBRemoteCommunication::ScopedTimeout timeout(
 *m_gdb_client_up, std::chrono::seconds(5));
-arg_packet_err = m_gdb_client_up->SendArgumentsPacket(launch_info);
+// Since we can't send argv0 separate from the executable path, we need to
+// make sure to use the actual executable path found in the launch_info...
+Args args = launch_info.GetArguments();
+if (FileSpec exe_file = launch_info.GetExecutableFile())
+  args.ReplaceArgumentAtIndex(0, exe_file.GetPath(false));
+if (llvm::Error err = m_gdb_client_up->LaunchProcess(args)) {
+  error.SetErrorStringWithFormatv("Cannot launch '{0}': {1}",
+  args.GetArgumentAtIndex(0),
+  llvm::fmt_consume(std::move(err)));
+  return error;
+}
   }
 
-  if (arg_packet_err == 0) {
-std::string error_str;
-if (m_gdb_client_up->GetLaunchSuccess(error_str)) {
-  const auto pid = m_gdb_client_up->GetCurrentProcessID(false);
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-launch_info.SetProcessID(pid);
-LLDB_LOGF(log,
-  "PlatformRemoteGDBServer::%s() pid %" PRIu64
-  " launched successfully",
-  __FUNCTION__, pid);
-  } else {
-LLDB_LOGF(log,
-  "PlatformRemoteGDBServer::%s() launch succeeded but we "
-  "didn't get a valid process id back!",
-  __FUNCTION__);
-error.SetErrorString("failed to get PID");
-  }
-} else {
-  error.SetErrorString(error_str.c_str());
-  LLDB_LOGF(log, "PlatformRemoteGDBServer::%s() launch failed: %s",
-__FUNCTION__, error.AsCString());
-}
+  const auto pid = m_gdb_client_up->GetCurrentProcessID(false);
+  if (pid != LLDB_INVALID_PROCESS_ID) {
+launch_info.SetProcessID(pid);
+LLDB_LOGF(log,
+  "PlatformRemoteGDBServer::%s() pid %" PRIu64
+  " launched successfully",
+  __FUNCTION__, pid);
   } else {
-error.SetErrorStringWithFormat("'A' packet returned an error: %i",
-   arg_packet_err);
+LLDB_LOGF(log,
+  "PlatformRemoteGDBServer::%s() launch succeeded but we "
+  "didn't get a valid process id back!",
+  __FUNCTION__);
+error.SetErrorString("failed to get PID");
   }
   return error;
 }

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 7e4688030c5b2..f03fd4231c74a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -746,108 +746,69 @@ lldb::pid_t 

[Lldb-commits] [lldb] 08c4a67 - [lldb] Move breakpoint hit reset code to Target::CleanupProcess

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-10-06T17:18:51+02:00
New Revision: 08c4a6795ac40f14d8a4385d28bc4f266ba895f1

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

LOG: [lldb] Move breakpoint hit reset code to Target::CleanupProcess

This ensures it is run regardless of the method we use to initiate the
session (previous version did not handle connects), and it is the same
place that is used for resetting watchpoints.

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

Added: 


Modified: 
lldb/source/Target/Process.cpp
lldb/source/Target/Target.cpp
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 1a9befdfcb0ce..bbc5cb87f86f6 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2761,18 +2761,15 @@ ListenerSP 
ProcessAttachInfo::GetListenerForProcess(Debugger ) {
 }
 
 Status Process::WillLaunch(Module *module) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillLaunch(module);
 }
 
 Status Process::WillAttachToProcessWithID(lldb::pid_t pid) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillAttachToProcessWithID(pid);
 }
 
 Status Process::WillAttachToProcessWithName(const char *process_name,
 bool wait_for_launch) {
-  GetTarget().ResetBreakpointHitCounts();
   return DoWillAttachToProcessWithName(process_name, wait_for_launch);
 }
 

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 27c58cb6e2e16..c567407757e39 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -177,6 +177,7 @@ void Target::CleanupProcess() {
   // clean up needs some help from the process.
   m_breakpoint_list.ClearAllBreakpointSites();
   m_internal_breakpoint_list.ClearAllBreakpointSites();
+  ResetBreakpointHitCounts();
   // Disable watchpoints just on the debugger side.
   std::unique_lock lock;
   this->GetWatchpointList().GetListMutex(lock);

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
index 1e0a519f420fa..191eda5cfecbc 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
@@ -63,3 +63,57 @@ def test_process_connect_async(self):
 self.dbg.GetSelectedTarget().GetProcess().Kill()
 lldbutil.expect_state_changes(self, self.dbg.GetListener(),
   self.process(), [lldb.eStateExited])
+def test_breakpoint_count(self):
+"""
+Test that breakpoint count gets reset for each new connection.
+"""
+class MyResponder(MockGDBServerResponder):
+
+def __init__(self):
+super().__init__()
+self.continued = False
+
+def qfThreadInfo(self):
+return "m47"
+
+def qsThreadInfo(self):
+return "l"
+
+def setBreakpoint(self, packet):
+return "OK"
+
+def readRegister(self, reg):
+# Pretend we're at the breakpoint after we've been resumed.
+return "3412" if self.continued else 
"4747"
+
+def cont(self):
+self.continued = True
+return "T05thread=47;reason:breakpoint"
+
+# Connect to the first process and set our breakpoint.
+self.server.responder = MyResponder()
+target = self.createTarget("a.yaml")
+process = self.connect(target)
+
+bkpt = target.BreakpointCreateByAddress(0x1234)
+self.assertTrue(bkpt.IsValid())
+self.assertEqual(bkpt.GetNumLocations(), 1)
+
+# "continue" the process. It should hit our breakpoint.
+process.Continue()
+self.assertState(process.GetState(), lldb.eStateStopped)
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Now kill it. The breakpoint should still show a hit count of one.
+process.Kill()
+self.server.stop()
+self.assertEqual(bkpt.GetHitCount(), 1)
+
+# Start over, and reconnect.
+self.server = MockGDBServer(self.server_socket_class())
+self.server.start()
+
+process = self.connect(target)
+
+# The hit count should be reset.
+self.assertEqual(bkpt.GetHitCount(), 0)



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

[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

[Pavel Labath via Phabricator via lldb-commits]

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

Let's just skip this check. Though it might be fun to implement an 
emptyness-of-intersection check for two regular expressions, that's: a) 
overkill; b) impossible if you include backreferences.


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

https://reviews.llvm.org/D134570

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

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

[David Spickett via Phabricator via lldb-commits]

DavidSpickett abandoned this revision.
DavidSpickett added a comment.

https://reviews.llvm.org/D135015 landed using std::variant and no one has 
complained so far, so this whole patch is now irrelevant. If it comes back, 
I'll just use variant.

I agree that there are reasons to keep things POD so I am working on that basis 
with the goal of being able to process information sent to us by remote stubs.

Core files and information provided by lldb itself, I'm thinking about in the 
background so thanks for the use cases there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134041

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

[Lldb-commits] [PATCH] D134771: [NFCI] Simplify TypeCategoryImpl for-each callbacks.

[Pavel Labath via Phabricator via lldb-commits]

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

(it looks like I did not click "submit" for some reason)




Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:169
+template  ForEachCallback(Callable c) : callback(c) {}
+std::function)> 
callback;
   };

And one reference here as well.



Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:159-162
+  // TypeFilterImpl inherits from SyntheticChildren, so we can't simply 
overload
+  // ForEach on the type of the callback because it would result in "call to
+  // member function 'ForEach' is ambiguous" errors. Instead we use this
+  // templated struct to hold the formatter type and the callback.

jgorbe wrote:
> labath wrote:
> > What if we just embed the type information into the method name? (So we 
> > could have a set of `ForEachFormat`,`ForEachSummary`, ... methods instead 
> > of a single overloaded ForEach)
> The problem with that is that the call site is
> 
> ```
> template 
> class CommandObjectTypeFormatterList {
>   [...]
>   bool DoExecute(...) {
>   TypeCategoryImpl::ForEachCallbacks foreach;
>   category->ForEach(foreach);
> ```
> 
> So if we want to keep that template for `CommandObjectTypeFormatterList` to 
> avoid repeating the implementation of `type {format, summary, filter, 
> synthetic} list`, we still need to switch based on type //somewhere//. So it 
> might as well be here. Or did you have anything else in mind?
No, this is what I had in mind, but I somehow missed the fact that the call 
site is templated. Ok, let's stick to this then.


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

https://reviews.llvm.org/D134771

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

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:331
+ clang::DeclContext *parent_decl_ctx) {
+  static lldb::user_id_t anonymous_id = LLDB_INVALID_UID - 1;
+  clang::FieldDecl *field_decl = nullptr;

Now multiple symbol files can race when accessing this variable (and this also 
introduces a strange interaction between two supposedly-independent symbol 
files). Better make this member variable of the parent symbol file, or 
something like that.



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:392-443
+  // For anonymous unions in a struct, msvc generated pdb doesn't have the
+  // entity for that union. So, we need to construct anonymous union and struct
+  // based on field offsets. The final AST is likely not matching the exact
+  // original AST, but the memory layout is preseved.
+
+  // The end offset to a field/struct/union that ends at the offset.
+  std::map end_offset_map;

This could use a high-level explanation of the algorithm. Like, I know it tries 
to stuff the fields into structs and unions, but I wasn't able to figure out 
how it does that, and what are the operating invariants.

The thing I like about this algorithm, is that the most complicated part (the 
thing I highlighted) is basically just playing with numbers and it is 
independent of any complex objects and state. If this part is separated out 
into a separate function, then it would be perfectly suitable for unit testing, 
and the unit tests could also serve as documentation/examples of the kinds of 
transformations that the algorithm does.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:54
+  struct Field {
+enum Kind { FIELD, STRUCT, UNION } kind;
+// Following are only used for field.

according to 
,
 these should be called `Field, Struct, Union`



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:64
+uint64_t base_offset;
+llvm::SmallVector fields;
+

Can we drop the `SP` part? I think that the owning references (I guess that's 
this field) could be just plain Field instances (unique_ptr at worst), 
and the rest could just be plain pointers and references.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.h:92
+  // llvm::SmallVector m_fields;
+  uint64_t start_offset = UINT64_MAX;
+  bool m_is_struct;

m_start_offset ?



Comment at: lldb/test/Shell/SymbolFile/NativePDB/class_layout.cpp:53-59
+// CHECK-NEXT: | |-DefinitionData pass_in_registers aggregate standard_layout 
trivially_copyable pod trivial literal
+// CHECK-NEXT: | | |-DefaultConstructor exists trivial needs_implicit
+// CHECK-NEXT: | | |-CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveConstructor exists simple trivial needs_implicit
+// CHECK-NEXT: | | |-CopyAssignment simple trivial has_const_param 
needs_implicit implicit_has_const_param
+// CHECK-NEXT: | | |-MoveAssignment exists simple trivial needs_implicit
+// CHECK-NEXT: | | `-Destructor simple irrelevant trivial needs_implicit

I think we should exclude all of these DefinitionData fields from the test 
expectation, as they are largely irrelevant to the test (and they also 
obfuscate it very well). Otherwise, people will have to update these whenever a 
new field gets added.

I don't know whether it contains the thing you wanted to test (as I don't know 
what that is), but the `type lookup C` output will contain the general 
structure of the type, and it will be a lot more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134849

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

[Lldb-commits] [PATCH] D134965: [LLDB] Change RegisterValue::GetAsMemoryData to const RegisterInfo

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134965

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

[Lldb-commits] [PATCH] D134963: [LLDB] Switch to RegisterInfo& for EmulateInstruction::WriteRegister

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134963

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

[Lldb-commits] [PATCH] D134962: [LLDB] Change pointer to ref in EmulateInstruction::ReadRegister methods

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134962

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

[Lldb-commits] [PATCH] D135352: [LLDB] Complete set of char tests for static integral members

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Previously we had a bit of a mix of "signed char" "unsigned char" and
"char".

This adds seperate min and max checks for all three types.

Depends on D135170 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135352

Files:
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -26,7 +26,8 @@
   const static int int_val_with_address = 2;
   const static bool bool_val = true;
 
-  const static auto char_max = std::numeric_limits::max();
+  const static auto char_max = std::numeric_limits::max();
+  const static auto schar_max = std::numeric_limits::max();
   const static auto uchar_max = std::numeric_limits::max();
   const static auto int_max = std::numeric_limits::max();
   const static auto uint_max = std::numeric_limits::max();
@@ -37,6 +38,7 @@
   std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
+  const static auto schar_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
   const static auto int_min = std::numeric_limits::min();
   const static auto uint_min = std::numeric_limits::min();
@@ -83,6 +85,7 @@
   A a;
 
   auto char_max = A::char_max;
+  auto schar_max = A::schar_max;
   auto uchar_max = A::uchar_max;
   auto int_max = A::int_max;
   auto uint_max = A::uint_max;
@@ -92,6 +95,7 @@
   auto ulonglong_max = A::ulonglong_max;
 
   auto char_min = A::char_min;
+  auto schar_min = A::schar_min;
   auto uchar_min = A::uchar_min;
   auto int_min = A::int_min;
   auto uint_min = A::uint_min;
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -34,6 +34,7 @@
 
 # Test that minimum and maximum values for each data type are right.
 self.expect_expr("A::char_max == char_max", result_value="true")
+self.expect_expr("A::schar_max == schar_max", result_value="true")
 self.expect_expr("A::uchar_max == uchar_max", result_value="true")
 self.expect_expr("A::int_max == int_max", result_value="true")
 self.expect_expr("A::uint_max == uint_max", result_value="true")
@@ -43,6 +44,7 @@
 self.expect_expr("A::ulonglong_max == ulonglong_max", 
result_value="true")
 
 self.expect_expr("A::char_min == char_min", result_value="true")
+self.expect_expr("A::schar_min == schar_min", result_value="true")
 self.expect_expr("A::uchar_min == uchar_min", result_value="true")
 self.expect_expr("A::int_min == int_min", result_value="true")
 self.expect_expr("A::uint_min == uint_min", result_value="true")


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -26,7 +26,8 @@
   const static int int_val_with_address = 2;
   const static bool bool_val = true;
 
-  const static auto char_max = std::numeric_limits::max();
+  const static auto char_max = std::numeric_limits::max();
+  const static auto schar_max = std::numeric_limits::max();
   const static auto uchar_max = std::numeric_limits::max();
   const static auto int_max = std::numeric_limits::max();
   const static auto uint_max = std::numeric_limits::max();
@@ -37,6 +38,7 @@
   std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
+  const static auto schar_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
   const static auto int_min = std::numeric_limits::min();
   const static auto uint_min = std::numeric_limits::min();
@@ -83,6 +85,7 @@
   A a;
 
   auto char_max = A::char_max;
+  auto schar_max = A::schar_max;
   auto uchar_max = A::uchar_max;
   auto int_max = A::int_max;
   auto uint_max = A::uint_max;
@@ -92,6 +95,7 @@
   auto ulonglong_max = A::ulonglong_max;
 
   auto char_min = A::char_min;
+  auto schar_min = A::schar_min;
   auto uchar_min = A::uchar_min;
   auto int_min = A::int_min;
   auto uint_min = A::uint_min;
Index: lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py:1
+"""
+Test that we return only the requested template instantiation.

I think it would be good to test this with 
`-gsimple-template-names`/`-gno-simple-template-names`

You can pass in a dictionary into `self.build()` and override Makefile flags

E..g, in `lldb/test/API/lang/c/forward/TestForwardDeclaration.py` we do this 
for `-gdwarf-5`.

So you could have two `test_XXX` methods like:
```
def do_test(self, debug_flags):
self.build(dictionary=debug_flags)
... actual test ...

def test_simple_template_name(self):
do_test(self, dict(CFLAGS_EXTRAS="-gsimple-template-names")

def test_no_simple_template_name(self):
do_test(self, dict(CFLAGS_EXTRAS="-gno-simple-template-names")
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1528
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE ) {
+  if (llvm::StringRef(die.GetName()).contains("<"))
+return std::string();

Michael137 wrote:
> Michael137 wrote:
> > Can this affect `operator<` at all? Haven't thought much about it but 
> > that's one construct that comes to mind that can contain `<`
> > 
> > ```
> > $ cat operator.cpp 
> > template
> > struct Foo {
> > int x = 0;
> > 
> > template
> > friend bool operator<(Foo const& lhs, Foo const& rhs) {
> > return lhs.x < rhs.x;
> > }
> > };
> > 
> > int main() {
> > Foo x;
> > Foo y;
> > return x < y;
> > }
> > ```
> > 
> > ```
> > 0x00c8:   DW_TAG_subprogram
> > DW_AT_low_pc(0x00013f5c)   
> > DW_AT_high_pc   (0x00013f8c)   
> > DW_AT_APPLE_omit_frame_ptr  (true)
> > DW_AT_frame_base(DW_OP_reg31 WSP)  
> > DW_AT_linkage_name  ("_ZltIiEbRK3FooIT_ES4_")  
> > DW_AT_name  ("operator<") 
> > DW_AT_decl_file ("/Users/michaelbuch/operator.cpp")
> > DW_AT_decl_line (6)
> > DW_AT_type  (0x0135 "bool")
> > DW_AT_external  (true) 
> > ```
> Looks like `-gsimple-template-names` leaves the template params in 
> `DW_AT_name` in this case so we're fine? Would be nice to test though
Oh this wouldn't apply anyway since we only care about type names, ignore this 
comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1528
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE ) {
+  if (llvm::StringRef(die.GetName()).contains("<"))
+return std::string();

Michael137 wrote:
> Can this affect `operator<` at all? Haven't thought much about it but that's 
> one construct that comes to mind that can contain `<`
> 
> ```
> $ cat operator.cpp 
> template
> struct Foo {
> int x = 0;
> 
> template
> friend bool operator<(Foo const& lhs, Foo const& rhs) {
> return lhs.x < rhs.x;
> }
> };
> 
> int main() {
> Foo x;
> Foo y;
> return x < y;
> }
> ```
> 
> ```
> 0x00c8:   DW_TAG_subprogram
> DW_AT_low_pc(0x00013f5c)   
> DW_AT_high_pc   (0x00013f8c)   
> DW_AT_APPLE_omit_frame_ptr  (true)
> DW_AT_frame_base(DW_OP_reg31 WSP)  
> DW_AT_linkage_name  ("_ZltIiEbRK3FooIT_ES4_")  
> DW_AT_name  ("operator<") 
> DW_AT_decl_file ("/Users/michaelbuch/operator.cpp")
> DW_AT_decl_line (6)
> DW_AT_type  (0x0135 "bool")
> DW_AT_external  (true) 
> ```
Looks like `-gsimple-template-names` leaves the template params in `DW_AT_name` 
in this case so we're fine? Would be nice to test though


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D134378: [lldb] Support simplified template names

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1528
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE ) {
+  if (llvm::StringRef(die.GetName()).contains("<"))
+return std::string();

Can this affect `operator<` at all? Haven't thought much about it but that's 
one construct that comes to mind that can contain `<`

```
$ cat operator.cpp 
template
struct Foo {
int x = 0;

template
friend bool operator<(Foo const& lhs, Foo const& rhs) {
return lhs.x < rhs.x;
}
};

int main() {
Foo x;
Foo y;
return x < y;
}
```

```
0x00c8:   DW_TAG_subprogram
DW_AT_low_pc(0x00013f5c)   
DW_AT_high_pc   (0x00013f8c)   
DW_AT_APPLE_omit_frame_ptr  (true)
DW_AT_frame_base(DW_OP_reg31 WSP)  
DW_AT_linkage_name  ("_ZltIiEbRK3FooIT_ES4_")  
DW_AT_name  ("operator<") 
DW_AT_decl_file ("/Users/michaelbuch/operator.cpp")
DW_AT_decl_line (6)
DW_AT_type  (0x0135 "bool")
DW_AT_external  (true) 
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134378

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

[Lldb-commits] [PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39
 
   const static auto char_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();

shafik wrote:
> We use `signed char` for the max case but `char` for the min case. Maybe we 
> need a max using `char` and a min using `signed char`.
I'll look into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135170

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