[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetCPlusPlusQualifiedName

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1533
 
+std::string
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {

Ah the function was declared but not defined (presumably a leftover from 
https://reviews.llvm.org/D138834)

Can we elaborate in the function docstring how this differs from 
`GetTemplateParameters`?



Comment at: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1579-1580
   while (parent_decl_ctx_die) {
 // The name may not contain template parameters due to simplified template
 // names; we must reconstruct the full name from child template parameter
 // dies via GetTemplateParametersString().

While you're here mind correcting this?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:101
+  /// per DIE, we need to start caching the results to prevent unbounded growth
+  /// of the created clang AST nodes.
   ///

Btw, what's guaranteeing that we don't call this more than once per DIE? Is 
there any way we could add a test that would notify us of that kind of 
regression? Or perhaps an assert somewhere in the `DWARFASTParserClang`?



Comment at: lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py:13
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", 
lldb.SBFileSpec("main.cpp"))
+self.expect("image lookup -A -t 'Inner'", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])

Nit: technically don't need to run the program to test `image lookup`?



Comment at: lldb/test/API/lang/cpp/nested-template/main.cpp:10
+  Outer::Inner oi;
+  // Set breakpoint here
+}

Nit: we typically use `std::puts("Set breakpoint here")` or `return 0` for 
source regex breakpoints. The rationale was that breaking on comments isn't 
always reliable/future-proof.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

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

[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetCPlusPlusQualifiedName

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D142413#4075198 , @aeubanks wrote:

> there may be an alternate solution involving making some declarations into 
> definitions (at all? earlier?), but I don't have that level of understanding 
> of lldb

One idea that came to mind is that instead of creating a dummy template decl in 
`GetDIEClassTemplateParams`, you could perhaps construct a 
`clang::TemplateArgumentList` from the `TemplateArgument`s stored in 
`TemplateParameterInfos` and use the `clang::printTemplateArgumentList` to 
stringify the template arguments, passing it the printing-policy which is used 
in `TypeSystemClang` to print type-names in your current implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

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

[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetCPlusPlusQualifiedName

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1533
 
+std::string
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {

Michael137 wrote:
> Ah the function was declared but not defined (presumably a leftover from 
> https://reviews.llvm.org/D138834)
> 
> Can we elaborate in the function docstring how this differs from 
> `GetTemplateParameters`?
Meant to say `GetDIEClassTemplateParams`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

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

[Lldb-commits] [PATCH] D142266: [lldb] Add PlatformMetadata for ScriptedPlatform

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Friendly reminding @labath @JDevlieghere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142266

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

[Lldb-commits] [PATCH] D138618: [LLDB] Enable 64 bit debug/type offset

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

In D138618#4073329 , @labath wrote:

> In D138618#4060565 , @clayborg 
> wrote:
>
>> ...
>> Since the user IDs of SymbolFileDWARF plug-ins mean nothing to anyone else, 
>> we can make them what we need them to be so they work for us. I would 
>> suggest to remove the use of DIERef from calculating the IDs of symbol files 
>> and have .o files for mac and .dwo files for fission use a 1 based index as 
>> their ID to make it easy to encode into a DIERef when needed for 
>> lldb::user_id_t values that _are_ included in objects that we hand out. Is 
>> there anything else that would need to be done to keep everyone happy here?
>
> I think that the 1-based index thingy helps a lot here, but I haven't seen 
> anything (in your reponse, or in the new patch) that would address my 
> concernt DIERef<->user_id conversion ambiguity. I.e. how is one supposed to 
> know what is the "right" way to convert a DIERef to a user_id:
>
> - `die_ref.get_id()`
> - or `symbol_file.GetUID(die_ref)` (which, funnily enough, will construct 
> another DIERef, and *then* call get_id? (`return DIERef(GetID(), 
> ref.section(), ref.die_offset()).get_id();`)
>
> What's your position on that? That we should live with the ambiguity?

Searching for GetUID doesn't look like it's used all that often, maybe follow 
up patch is just to get rid of it, and replace with DIERef?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138618

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

[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetCPlusPlusQualifiedName

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks updated this revision to Diff 491924.
aeubanks marked an inline comment as done.
aeubanks added a comment.

use suggested alternative


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/nested-template/Makefile
  lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
  lldb/test/API/lang/cpp/nested-template/main.cpp

Index: lldb/test/API/lang/cpp/nested-template/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/nested-template/main.cpp
@@ -0,0 +1,10 @@
+struct Outer {
+  Outer() {}
+
+  template 
+  struct Inner {};
+};
+
+int main() {
+  Outer::Inner oi;
+}
Index: lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
@@ -0,0 +1,24 @@
+"""
+Test that a nested template parameter works with simple template names.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class NestedTemplateTestCase(TestBase):
+def do_test(self, debug_flags):
+self.build(dictionary=debug_flags)
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.expect("image lookup -A -t 'Inner'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+self.do_test(dict(TEST_CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_names(self):
+self.do_test(dict(TEST_CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/nested-template/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/nested-template/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp
+
+CFLAGS_EXTRAS = $(TEST_CFLAGS_EXTRAS) $(LIMIT_DEBUG_INFO_FLAGS)
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1067,6 +1067,10 @@
 
   bool SetDeclIsForcefullyCompleted(const clang::TagDecl *td);
 
+  /// Return the template parameters (including surrounding <>) in string form.
+  std::string
+  PrintTemplateParams(const TemplateParameterInfos &template_param_infos);
+
 private:
   /// Returns the PrintingPolicy used when generating the internal type names.
   /// These type names are mostly used for the formatter selection.
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1431,6 +1431,24 @@
   return template_param_list;
 }
 
+std::string TypeSystemClang::PrintTemplateParams(
+const TemplateParameterInfos &template_param_infos) {
+  llvm::SmallVector ignore;
+  clang::TemplateParameterList *template_param_list =
+  CreateTemplateParameterList(getASTContext(), template_param_infos,
+  ignore);
+  llvm::SmallVector args =
+  template_param_infos.args;
+  if (template_param_infos.hasParameterPack()) {
+args.append(template_param_infos.packed_args->args);
+  }
+  std::string str;
+  llvm::raw_string_ostream os(str);
+  clang::printTemplateArgumentList(os, args, GetTypePrintingPolicy(),
+   template_param_list);
+  return str;
+}
+
 clang::FunctionTemplateDecl *TypeSystemClang::CreateFunctionTemplateDecl(
 clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
 clang::FunctionDecl *func_decl,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -95,11 +95,6 @@
   /// parameters from the DIE name and instead always adds template parameter
   /// children DIEs.
   ///
-  /// Currently this is only called in two places, when uniquing C++ classes and
-  /// when looking up the definition for a declaration (which is then cached).
-  /// If this is ever called more than twice per DIE, we need to start caching
-  /// t

[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetDIEClassTemplateParams

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added a comment.

In D142413#4076749 , @Michael137 
wrote:

> In D142413#4075198 , @aeubanks 
> wrote:
>
>> there may be an alternate solution involving making some declarations into 
>> definitions (at all? earlier?), but I don't have that level of understanding 
>> of lldb
>
> One idea that came to mind is that instead of creating a dummy template decl 
> in `GetDIEClassTemplateParams`, you could perhaps construct a 
> `clang::TemplateArgumentList` from the `TemplateArgument`s stored in 
> `TemplateParameterInfos` and use the `clang::printTemplateArgumentList` to 
> stringify the template arguments, passing it the printing-policy which is 
> used in `TypeSystemClang` to print type-names in your current implementation.

this worked perfectly and simplified a lot of code, thanks!




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1533
 
+std::string
+DWARFASTParserClang::GetTemplateParametersString(const DWARFDIE &die) {

Michael137 wrote:
> Michael137 wrote:
> > Ah the function was declared but not defined (presumably a leftover from 
> > https://reviews.llvm.org/D138834)
> > 
> > Can we elaborate in the function docstring how this differs from 
> > `GetTemplateParameters`?
> Meant to say `GetDIEClassTemplateParams`
yeah I forgot to remove the declaration in that patch :|



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h:101
+  /// per DIE, we need to start caching the results to prevent unbounded growth
+  /// of the created clang AST nodes.
   ///

Michael137 wrote:
> Btw, what's guaranteeing that we don't call this more than once per DIE? Is 
> there any way we could add a test that would notify us of that kind of 
> regression? Or perhaps an assert somewhere in the `DWARFASTParserClang`?
now we don't create a clang AST node



Comment at: lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py:13
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", 
lldb.SBFileSpec("main.cpp"))
+self.expect("image lookup -A -t 'Inner'", 
DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])

Michael137 wrote:
> Nit: technically don't need to run the program to test `image lookup`?
last time I couldn't find the right thing to call to load the binary but not 
set a breakpoint, but perhaps this works?



Comment at: lldb/test/API/lang/cpp/nested-template/main.cpp:10
+  Outer::Inner oi;
+  // Set breakpoint here
+}

Michael137 wrote:
> Nit: we typically use `std::puts("Set breakpoint here")` or `return 0` for 
> source regex breakpoints. The rationale was that breaking on comments isn't 
> always reliable/future-proof.
obsolete now that we don't set a breakpoint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

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

[Lldb-commits] [PATCH] D142513: [lldb][test] Set minimum compiler_versions

[Dave Lee via Phabricator via lldb-commits]

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

Set compiler_versions on these tests, as they fail if tested on lower compiler 
versions versions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142513

Files:
  
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
  lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,6 +1,6 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '16.0'])]
 lldbinline.MakeInlineTest(
 __file__, globals(), decor)
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
@@ -28,7 +28,8 @@
 children=self.check_string_vec_children())
 
 @add_test_categories(["libc++"])
-@skipIf(compiler=no_match("clang"), compiler_version=['<', '16.0'])
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '16.0'])
 def test_with_run_command(self):
 """Test that std::ranges::ref_view is formatted correctly when printed.
 """
Index: 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -12,6 +12,7 @@
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '12.0'])
 def test(self):
 self.build()
 
Index: 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
@@ -12,6 +12,7 @@
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '12.0'])
 def test(self):
 self.build()
 
Index: 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
===
--- 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
+++ 
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
@@ -11,6 +11,7 @@
 
 @add_test_categories(["libc++"])
 @skipIf(compiler=no_match("clang"))
+@skipIf(compiler="clang", compiler_version=['<', '12.0'])
 def test(self):
 self.build()
 


Index: lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
===
--- lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
+++ lldb/test/API/lang/objc/objc_direct-methods/TestObjCDirectMethods.py
@@ -1,6 +1,6 @@
 from lldbsuite.test import lldbinline
 from lldbsuite.test import decorators
 
-decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '13.0'])]
+decor = [decorators.skipIf(compiler="clang", compiler_version=['<', '16.0'])]
 lldbinline.MakeInlineTest(
 __file__, globals(), decor)
Index:

[Lldb-commits] [PATCH] D142513: [lldb][test] Set minimum compiler_versions

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

These are failing on green dragon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142513

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

[Lldb-commits] [PATCH] D142513: [lldb][test] Set minimum compiler_versions

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D142513#4078690 , @kastiglione 
wrote:

> These are failing on green dragon.

Which job is failing? GreenDragon looks green to me so I must be looking in the 
wrong places. But also isn't green dragon using the current ToT compiler to 
build the tests?

Do you know what change in the compiler is causing these test failures? Are 
they all the same?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142513

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

[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetDIEClassTemplateParams

[Michael Buch via Phabricator via lldb-commits]

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

awesome, looks much better now
thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

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

[Lldb-commits] [PATCH] D142513: [lldb][test] Set minimum compiler_versions

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

It's the LLDB matrix that's failing, where previous compiler versions are
also used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142513

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

[Lldb-commits] [lldb] 5dd7c16 - [lldb] Don't create Clang AST nodes in GetDIEClassTemplateParams

[Arthur Eubanks via lldb-commits]

Author: Arthur Eubanks
Date: 2023-01-24T19:13:19-08:00
New Revision: 5dd7c16c3dcfd3154a53ee59e0e092c1e0092197

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

LOG: [lldb] Don't create Clang AST nodes in GetDIEClassTemplateParams

Otherwise we may be inserting a decl into a DeclContext that's not fully 
defined yet.

This simplifies/removes some clang AST node creation code. Instead, use
clang::printTemplateArgumentList().

Reviewed By: Michael137

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

Added: 
lldb/test/API/lang/cpp/nested-template/Makefile
lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
lldb/test/API/lang/cpp/nested-template/main.cpp

Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 9ebc49417138b..56bd089b86a1e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -745,28 +745,9 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const 
DWARFDIE &die) {
   if (llvm::StringRef(die.GetName()).contains("<"))
 return ConstString();
 
-  clang::DeclContext *decl_ctx = GetClangDeclContextContainingDIE(die, 
nullptr);
   TypeSystemClang::TemplateParameterInfos template_param_infos;
   if (ParseTemplateParameterInfos(die, template_param_infos)) {
-// Most of the parameters here don't matter, but we make sure the base name
-// is empty so when we print the name we only get the template parameters.
-clang::ClassTemplateDecl *class_template_decl =
-m_ast.ParseClassTemplateDecl(decl_ctx, GetOwningClangModule(die),
- eAccessPublic, "", clang::TTK_Struct,
- template_param_infos);
-if (!class_template_decl)
-  return ConstString();
-
-clang::ClassTemplateSpecializationDecl *class_specialization_decl =
-m_ast.CreateClassTemplateSpecializationDecl(
-decl_ctx, GetOwningClangModule(die), class_template_decl,
-clang::TTK_Struct, template_param_infos);
-if (!class_specialization_decl)
-  return ConstString();
-CompilerType clang_type =
-m_ast.CreateClassTemplateSpecializationType(class_specialization_decl);
-ConstString name = clang_type.GetTypeName(/*BaseOnly*/ true);
-return name;
+return ConstString(m_ast.PrintTemplateParams(template_param_infos));
   }
   return ConstString();
 }
@@ -1541,9 +1522,9 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const 
DWARFDIE &die) {
   DWARFDIE parent_decl_ctx_die = die.GetParentDeclContextDIE();
   // TODO: change this to get the correct decl context parent
   while (parent_decl_ctx_die) {
-// The name may not contain template parameters due to simplified template
-// names; we must reconstruct the full name from child template parameter
-// dies via GetTemplateParametersString().
+// The name may not contain template parameters due to
+// -gsimple-template-names; we must reconstruct the full name from child
+// template parameter dies via GetDIEClassTemplateParams().
 const dw_tag_t parent_tag = parent_decl_ctx_die.Tag();
 switch (parent_tag) {
 case DW_TAG_namespace: {

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index c078d997df9f3..0733d22a0069b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -95,11 +95,6 @@ class DWARFASTParserClang : public DWARFASTParser {
   /// parameters from the DIE name and instead always adds template parameter
   /// children DIEs.
   ///
-  /// Currently this is only called in two places, when uniquing C++ classes 
and
-  /// when looking up the definition for a declaration (which is then cached).
-  /// If this is ever called more than twice per DIE, we need to start caching
-  /// the results to prevent unbounded growth of the created clang AST nodes.
-  ///
   /// \param die The struct/class DWARFDIE containing template parameters.
   /// \return A string, including surrounding '<>', of the template parameters.
   /// If the DIE's name already has '<>', returns an empty ConstString because
@@ -146,10 +141,6 @@ class DWARFASTParserClang : public DWARFASTParser {
   lldb_private::TypeSystemClang::TemplateParameterInfos
   &template_param_

[Lldb-commits] [PATCH] D142413: [lldb] Don't create Clang AST nodes in GetDIEClassTemplateParams

[Arthur Eubanks via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5dd7c16c3dcf: [lldb] Don't create Clang AST nodes in 
GetDIEClassTemplateParams (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142413

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/nested-template/Makefile
  lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
  lldb/test/API/lang/cpp/nested-template/main.cpp

Index: lldb/test/API/lang/cpp/nested-template/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/nested-template/main.cpp
@@ -0,0 +1,10 @@
+struct Outer {
+  Outer() {}
+
+  template 
+  struct Inner {};
+};
+
+int main() {
+  Outer::Inner oi;
+}
Index: lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/nested-template/TestNestedTemplate.py
@@ -0,0 +1,24 @@
+"""
+Test that a nested template parameter works with simple template names.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class NestedTemplateTestCase(TestBase):
+def do_test(self, debug_flags):
+self.build(dictionary=debug_flags)
+self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.expect("image lookup -A -t 'Inner'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+self.do_test(dict(TEST_CFLAGS_EXTRAS="-gsimple-template-names"))
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_no_simple_template_names(self):
+self.do_test(dict(TEST_CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/nested-template/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/nested-template/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp
+
+CFLAGS_EXTRAS = $(TEST_CFLAGS_EXTRAS) $(LIMIT_DEBUG_INFO_FLAGS)
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -1067,6 +1067,10 @@
 
   bool SetDeclIsForcefullyCompleted(const clang::TagDecl *td);
 
+  /// Return the template parameters (including surrounding <>) in string form.
+  std::string
+  PrintTemplateParams(const TemplateParameterInfos &template_param_infos);
+
 private:
   /// Returns the PrintingPolicy used when generating the internal type names.
   /// These type names are mostly used for the formatter selection.
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1431,6 +1431,24 @@
   return template_param_list;
 }
 
+std::string TypeSystemClang::PrintTemplateParams(
+const TemplateParameterInfos &template_param_infos) {
+  llvm::SmallVector ignore;
+  clang::TemplateParameterList *template_param_list =
+  CreateTemplateParameterList(getASTContext(), template_param_infos,
+  ignore);
+  llvm::SmallVector args =
+  template_param_infos.args;
+  if (template_param_infos.hasParameterPack()) {
+args.append(template_param_infos.packed_args->args);
+  }
+  std::string str;
+  llvm::raw_string_ostream os(str);
+  clang::printTemplateArgumentList(os, args, GetTypePrintingPolicy(),
+   template_param_list);
+  return str;
+}
+
 clang::FunctionTemplateDecl *TypeSystemClang::CreateFunctionTemplateDecl(
 clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module,
 clang::FunctionDecl *func_decl,
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -95,11 +95,6 @@
   /// parameters from the DIE name and instead always adds template parameter
   /// children DIEs.
   ///
-  /// Currently this is only called in two places, when uniquing C++ classes and
-  /// when looking up the definition for a declaration (which is then cached).
-  /// If this is ever called more 

[Lldb-commits] [lldb] 5ed6d99 - [lldb] Remove legacy six module for py2->py3

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2023-01-24T19:46:26-08:00
New Revision: 5ed6d99a8311e54762df3d40dfb1d12578aaa4f5

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

LOG: [lldb] Remove legacy six module for py2->py3

LLDB only supports Python3 now, so the `six` shim for Python2 is no longer 
necessary.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/bindings/python/CMakeLists.txt
lldb/cmake/modules/LLDBConfig.cmake

Removed: 
lldb/third_party/Python/module/six/LICENSE
lldb/third_party/Python/module/six/six.py



diff  --git a/lldb/bindings/python/CMakeLists.txt 
b/lldb/bindings/python/CMakeLists.txt
index 4f7941e6cff1d..c631faf52ac34 100644
--- a/lldb/bindings/python/CMakeLists.txt
+++ b/lldb/bindings/python/CMakeLists.txt
@@ -60,13 +60,6 @@ function(finish_swig_python swig_target 
lldb_python_bindings_dir lldb_python_tar
 DEPENDS ${lldb_python_bindings_dir}/lldb.py
 COMMENT "Python script sym-linking LLDB Python API")
 
-  if(NOT LLDB_USE_SYSTEM_SIX)
-add_custom_command(TARGET ${swig_target} POST_BUILD VERBATIM
-  COMMAND ${CMAKE_COMMAND} -E copy
-"${LLDB_SOURCE_DIR}/third_party/Python/module/six/six.py"
-"${lldb_python_target_dir}/../six.py")
-  endif()
-
   add_custom_command(TARGET ${swig_target} POST_BUILD VERBATIM
 COMMAND ${CMAKE_COMMAND} -E copy
   "${lldb_python_bindings_dir}/lldb.py"

diff  --git a/lldb/cmake/modules/LLDBConfig.cmake 
b/lldb/cmake/modules/LLDBConfig.cmake
index ebb1eec8464a6..ec06ba285f270 100644
--- a/lldb/cmake/modules/LLDBConfig.cmake
+++ b/lldb/cmake/modules/LLDBConfig.cmake
@@ -65,7 +65,6 @@ add_optional_dependency(LLDB_ENABLE_PYTHON "Enable Python 
scripting support in L
 add_optional_dependency(LLDB_ENABLE_LIBXML2 "Enable Libxml 2 support in LLDB" 
LibXml2 LIBXML2_FOUND VERSION 2.8)
 add_optional_dependency(LLDB_ENABLE_FBSDVMCORE "Enable libfbsdvmcore support 
in LLDB" FBSDVMCore FBSDVMCore_FOUND QUIET)
 
-option(LLDB_USE_SYSTEM_SIX "Use six.py shipped with system and do not install 
a copy of it" OFF)
 option(LLDB_USE_ENTITLEMENTS "When codesigning, use entitlements if available" 
ON)
 option(LLDB_BUILD_FRAMEWORK "Build LLDB.framework (Darwin only)" OFF)
 option(LLDB_NO_INSTALL_DEFAULT_RPATH "Disable default RPATH settings in 
binaries" OFF)

diff  --git a/lldb/third_party/Python/module/six/LICENSE 
b/lldb/third_party/Python/module/six/LICENSE
deleted file mode 100644
index e558f9d494ab3..0
--- a/lldb/third_party/Python/module/six/LICENSE
+++ /dev/null
@@ -1,18 +0,0 @@
-Copyright (c) 2010-2015 Benjamin Peterson
-
-Permission is hereby granted, free of charge, to any person obtaining a copy of
-this software and associated documentation files (the "Software"), to deal in
-the Software without restriction, including without limitation the rights to
-use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies 
of
-the Software, and to permit persons to whom the Software is furnished to do so,
-subject to the following conditions:
-
-The above copyright notice and this permission notice shall be included in all
-copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, 
FITNESS
-FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
-COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
-IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
-CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

diff  --git a/lldb/third_party/Python/module/six/six.py 
b/lldb/third_party/Python/module/six/six.py
deleted file mode 100644
index 0df2e259237d1..0
--- a/lldb/third_party/Python/module/six/six.py
+++ /dev/null
@@ -1,887 +0,0 @@
-"""Utilities for writing code that runs on Python 2 and 3"""
-
-# Copyright (c) 2010-2015 Benjamin Peterson
-#
-# Permission is hereby granted, free of charge, to any person obtaining a copy
-# of this software and associated documentation files (the "Software"), to deal
-# in the Software without restriction, including without limitation the rights
-# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-# copies of the Software, and to permit persons to whom the Software is
-# furnished to do so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice shall be included in 
all
-# copies or substantial portions of the Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR 

[Lldb-commits] [PATCH] D142140: [lldb] Remove legacy six module for py2->py3

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5ed6d99a8311: [lldb] Remove legacy six module for 
py2->py3 (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142140

Files:
  lldb/bindings/python/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/third_party/Python/module/six/LICENSE
  lldb/third_party/Python/module/six/six.py

Index: lldb/third_party/Python/module/six/six.py
===
--- lldb/third_party/Python/module/six/six.py
+++ /dev/null
@@ -1,887 +0,0 @@
-"""Utilities for writing code that runs on Python 2 and 3"""
-
-# Copyright (c) 2010-2015 Benjamin Peterson
-#
-# Permission is hereby granted, free of charge, to any person obtaining a copy
-# of this software and associated documentation files (the "Software"), to deal
-# in the Software without restriction, including without limitation the rights
-# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-# copies of the Software, and to permit persons to whom the Software is
-# furnished to do so, subject to the following conditions:
-#
-# The above copyright notice and this permission notice shall be included in all
-# copies or substantial portions of the Software.
-#
-# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-# SOFTWARE.
-
-from __future__ import absolute_import
-
-import functools
-import itertools
-import operator
-import sys
-import types
-
-__author__ = "Benjamin Peterson "
-__version__ = "1.10.0"
-
-
-# Useful for very coarse version differentiation.
-PY2 = sys.version_info[0] == 2
-PY3 = sys.version_info[0] == 3
-PY34 = sys.version_info[0:2] >= (3, 4)
-
-if PY3:
-string_types = str,
-integer_types = int,
-class_types = type,
-text_type = str
-binary_type = bytes
-
-MAXSIZE = sys.maxsize
-else:
-string_types = basestring,
-integer_types = (int, long)
-class_types = (type, types.ClassType)
-text_type = unicode
-binary_type = str
-
-if sys.platform.startswith("java"):
-# Jython always uses 32 bits.
-MAXSIZE = int((1 << 31) - 1)
-else:
-# It's possible to have sizeof(long) != sizeof(Py_ssize_t).
-class X(object):
-
-def __len__(self):
-return 1 << 31
-try:
-len(X())
-except OverflowError:
-# 32-bit
-MAXSIZE = int((1 << 31) - 1)
-else:
-# 64-bit
-MAXSIZE = int((1 << 63) - 1)
-del X
-
-
-def _add_doc(func, doc):
-"""Add documentation to a function."""
-func.__doc__ = doc
-
-
-def _import_module(name):
-"""Import module, returning the module after the last dot."""
-__import__(name)
-return sys.modules[name]
-
-
-class _LazyDescr(object):
-
-def __init__(self, name):
-self.name = name
-
-def __get__(self, obj, tp):
-result = self._resolve()
-setattr(obj, self.name, result)  # Invokes __set__.
-try:
-# This is a bit ugly, but it avoids running this again by
-# removing this descriptor.
-delattr(obj.__class__, self.name)
-except AttributeError:
-pass
-return result
-
-
-class MovedModule(_LazyDescr):
-
-def __init__(self, name, old, new=None):
-super(MovedModule, self).__init__(name)
-if PY3:
-if new is None:
-new = name
-self.mod = new
-else:
-self.mod = old
-
-def _resolve(self):
-return _import_module(self.mod)
-
-def __getattr__(self, attr):
-_module = self._resolve()
-value = getattr(_module, attr)
-setattr(self, attr, value)
-return value
-
-
-class _LazyModule(types.ModuleType):
-
-def __init__(self, name):
-super(_LazyModule, self).__init__(name)
-self.__doc__ = self.__class__.__doc__
-
-def __dir__(self):
-attrs = ["__doc__", "__name__"]
-attrs += [attr.name for attr in self._moved_attributes]
-return attrs
-
-# Subclasses should override this
-_moved_attributes = []
-
-
-class MovedAttribute(_LazyDescr):
-
-def __init__(self, name, old_mod, new_mod, old_attr=None, new_attr=None):
-super(MovedAttribute, self).__init__(name)
-if PY3:
-if new_mod is None:
-new_mod = name
-self.mod = new_mod
-if new_attr is None:

[Lldb-commits] [lldb] ff9c31b - [LLDB] Fix for libc++ atomic allowing modification of contained value

[Pavel Kosov via lldb-commits]

Author: Pavel Kosov
Date: 2023-01-25T10:39:50+03:00
New Revision: ff9c31b23b7635f2c97c5f9c33fd4e032b717ad0

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

LOG: [LLDB] Fix for libc++ atomic allowing modification of contained value

Reviewed By: clayborg

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

Added: 
lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile

lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp

Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
index 4eec79a278402..8b30e3fb27d95 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
@@ -139,7 +139,7 @@ 
lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::GetChildAtIndex(
 
 size_t lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
 GetIndexOfChildWithName(ConstString name) {
-  return formatters::ExtractIndexFromString(name.GetCString());
+  return name == "Value" ? 0 : UINT32_MAX;
 }
 
 SyntheticChildrenFrontEnd *

diff  --git 
a/lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile 
b/lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
new file mode 100644
index 0..564cbada74e08
--- /dev/null
+++ b/lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules

diff  --git 
a/lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py 
b/lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
new file mode 100644
index 0..d2bb27d8c7df2
--- /dev/null
+++ 
b/lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ std::atomic."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+q_value = frame0.FindVariable("Q")
+self.assertTrue(q_value.IsValid(), "Got the SBValue for val")
+inner_val = q_value.GetChildAtIndex(0)
+self.assertTrue(inner_val.IsValid(), "Got the SBValue for inner atomic 
val")
+result = inner_val.SetValueFromCString("42")
+self.assertTrue(result, "Setting val returned True.")
+result = inner_val.GetValueAsUnsigned()
+self.assertTrue(result == 42, "Got correct value (42)")

diff  --git 
a/lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp 
b/lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
new file mode 100644
index 0..60dc085d7d1f6
--- /dev/null
+++ b/lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}



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

[Lldb-commits] [PATCH] D140623: [LLDB] Fix for libc++ atomic allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGff9c31b23b76: [LLDB] Fix for libc++ atomic allowing 
modification of contained value (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140623

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
  lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
  lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
  lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp


Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}
Index: 
lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
===
--- /dev/null
+++ 
lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ std::atomic."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+q_value = frame0.FindVariable("Q")
+self.assertTrue(q_value.IsValid(), "Got the SBValue for val")
+inner_val = q_value.GetChildAtIndex(0)
+self.assertTrue(inner_val.IsValid(), "Got the SBValue for inner atomic 
val")
+result = inner_val.SetValueFromCString("42")
+self.assertTrue(result, "Setting val returned True.")
+result = inner_val.GetValueAsUnsigned()
+self.assertTrue(result == 42, "Got correct value (42)")
Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp
@@ -139,7 +139,7 @@
 
 size_t lldb_private::formatters::LibcxxStdAtomicSyntheticFrontEnd::
 GetIndexOfChildWithName(ConstString name) {
-  return formatters::ExtractIndexFromString(name.GetCString());
+  return name == "Value" ? 0 : UINT32_MAX;
 }
 
 SyntheticChildrenFrontEnd *


Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::atomic Q(1);
+return Q; // Set break point at this line.
+}
Index: lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/atomic/TestChangeValue.py
@@ -0,0 +1,48 @@
+"""
+Test change libc++ std::atomic values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772

[Lldb-commits] [lldb] 92f0e4c - [LLDB] Fixes summary formatter for libc++ map allowing modification of contained value

[Pavel Kosov via lldb-commits]

Author: Pavel Kosov
Date: 2023-01-25T10:48:04+03:00
New Revision: 92f0e4ccafacb61f7de93e7ef5bd4beb02047086

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

LOG: [LLDB] Fixes summary formatter for libc++ map allowing modification of 
contained value

Reviewed By: clayborg

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

Added: 
lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp

Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
index bf6c65c8d9c2..21dbd64feac5 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -397,18 +397,9 @@ 
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex(
   // at this point we have a valid
   // we need to copy current_sp into a new object otherwise we will end up with
   // all items named __value_
-  DataExtractor data;
-  Status error;
-  iterated_sp->GetData(data, error);
-  if (error.Fail()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
-  }
   StreamString name;
   name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  auto potential_child_sp = CreateValueObjectFromData(
-  name.GetString(), data, m_backend.GetExecutionContextRef(),
-  m_element_type);
+  auto potential_child_sp = iterated_sp->Clone(ConstString(name.GetString()));
   if (potential_child_sp) {
 switch (potential_child_sp->GetNumChildren()) {
 case 1: {

diff  --git a/lldb/test/API/python_api/value/change_values/libcxx/map/Makefile 
b/lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
new file mode 100644
index ..564cbada74e0
--- /dev/null
+++ b/lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules

diff  --git 
a/lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py 
b/lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
new file mode 100644
index ..087e4c8446f8
--- /dev/null
+++ b/lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
@@ -0,0 +1,51 @@
+"""
+Test change libc++ map values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ map."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), 
CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+val_value = frame0.FindVariable("M")
+self.assertTrue(val_value.IsValid(), "Got the SBValue for val")
+pair0 = val_value.GetChildMemberWithName("[0]")
+self.assertTrue(pair0.IsValid(), "Got the SBValue for [0]")
+self.assertTrue(pair0.GetNumChildren() == 2, "Got 2 children")
+pair0_second = pair0.GetChildMemberWithName("second")
+self.assertTrue(pair0_second.IsValid(), "Got the SBValue for 
[0].second")
+result = pair0_second.SetValueFromCString("12345")
+self.assertTrue(result, "Setting val returned True.")
+result = pair0_second.GetValueAsUnsigned()
+self.assertTrue(result == 12345, "Got correct value (12345)")

diff  --git a/lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp 
b/lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
new file mode 100644
index ..4e1ee213e0f3
--- /dev/null
+++ b/lldb/test/API/python_api/value/change_values

[Lldb-commits] [PATCH] D140624: [LLDB] Fixes summary formatter for libc++ map allowing modification of contained value

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG92f0e4ccafac: [LLDB] Fixes summary formatter for libc++ map 
allowing modification of… (authored by kpdev42).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140624

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
  lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
  lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp

Index: lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/main.cpp
@@ -0,0 +1,7 @@
+#include 
+
+int main()
+{
+std::map M = {{1,1},{2,2}};
+return M[1]; // Set break point at this line.
+}
Index: lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/TestChangeValue.py
@@ -0,0 +1,51 @@
+"""
+Test change libc++ map values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class LibcxxChangeValueTestCase(TestBase):
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+
+@add_test_categories(["libc++"])
+@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24772")
+def test(self):
+"""Test that we can change values of libc++ map."""
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+bkpt = self.target().FindBreakpointByID(
+lldbutil.run_break_set_by_source_regexp(
+self, "Set break point at this line."))
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# Get Frame #0.
+target = self.dbg.GetSelectedTarget()
+process = target.GetProcess()
+self.assertState(process.GetState(), lldb.eStateStopped)
+thread = lldbutil.get_stopped_thread(
+process, lldb.eStopReasonBreakpoint)
+self.assertTrue(
+thread.IsValid(),
+"There should be a thread stopped due to breakpoint condition")
+frame0 = thread.GetFrameAtIndex(0)
+self.assertTrue(frame0.IsValid(), "Got a valid frame.")
+
+val_value = frame0.FindVariable("M")
+self.assertTrue(val_value.IsValid(), "Got the SBValue for val")
+pair0 = val_value.GetChildMemberWithName("[0]")
+self.assertTrue(pair0.IsValid(), "Got the SBValue for [0]")
+self.assertTrue(pair0.GetNumChildren() == 2, "Got 2 children")
+pair0_second = pair0.GetChildMemberWithName("second")
+self.assertTrue(pair0_second.IsValid(), "Got the SBValue for [0].second")
+result = pair0_second.SetValueFromCString("12345")
+self.assertTrue(result, "Setting val returned True.")
+result = pair0_second.GetValueAsUnsigned()
+self.assertTrue(result == 12345, "Got correct value (12345)")
Index: lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
===
--- /dev/null
+++ lldb/test/API/python_api/value/change_values/libcxx/map/Makefile
@@ -0,0 +1,6 @@
+CXX_SOURCES := main.cpp
+
+USE_LIBCPP := 1
+
+CXXFLAGS_EXTRAS := -O0
+include Makefile.rules
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
@@ -397,18 +397,9 @@
   // at this point we have a valid
   // we need to copy current_sp into a new object otherwise we will end up with
   // all items named __value_
-  DataExtractor data;
-  Status error;
-  iterated_sp->GetData(data, error);
-  if (error.Fail()) {
-m_tree = nullptr;
-return lldb::ValueObjectSP();
-  }
   StreamString name;
   name.Printf("[%" PRIu64 "]", (uint64_t)idx);
-  auto potential_child_sp = CreateValueObjectFromData(
-  name.GetString(), data, m_backend.GetExecutionContextRef(),
-  m_element_type);
+  auto potential_child_sp = iterated_sp->Clone(ConstString(name.GetString()));
   if (potential_child_sp) {
 switch (potential_child_sp->GetNumChildren()) {
 case 1: {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits