[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D138259#3947410 , @mstorsjo wrote:

> This seems to have caused build errors with GCC:
>
>   ../tools/lldb/source/Symbol/CompilerType.cpp: In member function ‘bool 
> lldb_private::CompilerType::IsForcefullyCompleted() const’:
>   ../tools/lldb/source/Symbol/CompilerType.cpp:99:25: error: base operand of 
> ‘->’ has non-pointer type ‘const TypeSystemWP’ {aka ‘const 
> std::weak_ptr’}
>  99 | return m_type_system->IsForcefullyCompleted(m_type);
> | ^~
>
> Any clues about what’s missing?

Someone fixed the build issue for me! Things should be working


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138259

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

[Lldb-commits] [PATCH] D137900: Make only one function that needs to be implemented when searching for types.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:435
+  ///   match: "b::a", "c::b::a", "d::b::a", "e::f::b::a".
+  lldb::TypeSP FindFirstType(ConstString type_name, bool exact_match);
 

aprantl wrote:
> Why is this not taking a TypeQuery that wraps a name?
This function doesn't need to exist as long as the "TypeSP 
TypeQuery::FindFirstType(Module)" function is ok to have around.



Comment at: lldb/include/lldb/Core/Module.h:442
   ///
-  /// \param searched_symbol_files
-  /// Prevents one file from being visited multiple times.
-  void
-  FindTypes(llvm::ArrayRef pattern, LanguageSet languages,
-llvm::DenseSet _symbol_files,
-TypeMap );
-
-  lldb::TypeSP FindFirstType(const SymbolContext , ConstString type_name,
- bool exact_match);
-
-  /// Find types by name that are in a namespace. This function is used by the
-  /// expression parser when searches need to happen in an exact namespace
-  /// scope.
+  /// \param[in] search_first
+  /// If non-null, this module will be searched before any other

aprantl wrote:
> copy error?
yep!



Comment at: lldb/include/lldb/Core/ModuleList.h:373
+  ///   match: "b::a", "c::b::a", "d::b::a", "e::f::b::a".
+  lldb::TypeSP FindFirstType(Module *search_first, ConstString type_name,
+ bool exact_match) const;

aprantl wrote:
> same question — why is this not a TypeQuery?
Like in Module.h we can get rid of this as long as the convenience functions in 
TypeQuery::FindFirstType() are ok.



Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:90
+  /// \param context A valid vector of CompilerContext entries that describes
+  /// this delcaratiion context. The first entry in the vector is the parent of
+  /// the subsequent entry, so the top most entry is the global namespace.

aprantl wrote:
> declaration
will fix



Comment at: lldb/include/lldb/Symbol/CompilerDecl.h:93
+  void GetCompilerContext(
+  llvm::SmallVectorImpl ) const;
+

aprantl wrote:
> aprantl wrote:
> > Why can't this be a return value? The context objects are tiny.
> ping?
Will change!



Comment at: lldb/include/lldb/Symbol/Type.h:85
+  /// doing name lookups.
+  TypeQuery() = default;
+

aprantl wrote:
> Can we get rid of this now?
I believe so! I will change "default" to "delete".



Comment at: lldb/include/lldb/Symbol/Type.h:241
+  /// Add a language family to the list of languages that should produce a 
match.
+  void AddLanguage(lldb::LanguageType language);
+

aprantl wrote:
> Is this necessary, or could this be rolled into the constructor?
I will add it to the constructor.



Comment at: lldb/include/lldb/Symbol/Type.h:306
+  /// more complete are are used when lookup up types in a clang module's debug
+  /// information.
+  bool m_module_search = false;

aprantl wrote:
> This sentence is missing at least one word. (I also don't quite understand 
> the purpose of this flag)
I will reword


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137900

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

[Lldb-commits] [PATCH] D138638: Report which modules have forcefully completed types in statistics.

[Greg Clayton via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc743f034a34: Report which modules have forcefully completed 
types in statistics. (authored by clayborg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138638

Files:
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/include/lldb/Target/Statistics.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Target/Statistics.cpp
  lldb/test/API/commands/statistics/basic/TestStats.py
  lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py

Index: lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
===
--- lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
+++ lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py
@@ -30,6 +30,23 @@
 self._check_type(target, "InheritsFromOne")
 self._check_type(target, "InheritsFromTwo")
 
+# Check that the statistics show that we had incomplete debug info.
+stats = self.get_stats()
+# Find the a.out module info in the stats and verify it has the
+# "debugInfoHadIncompleteTypes" key value pair set to True
+exe_module_found = False
+for module in stats['modules']:
+if module['path'].endswith('a.out'):
+self.assertTrue(module['debugInfoHadIncompleteTypes'])
+exe_module_found = True
+break
+self.assertTrue(exe_module_found)
+# Verify that "totalModuleCountWithIncompleteTypes" at the top level
+# is greater than zero which shows we had incomplete debug info in a
+# module
+self.assertGreater(stats['totalModuleCountWithIncompleteTypes'], 0)
+
+
 def _check_incomplete_frame_variable_output(self):
 # Check that the display of the "frame variable" output identifies the
 # incomplete types. Currently the expression parser will find the real
Index: lldb/test/API/commands/statistics/basic/TestStats.py
===
--- lldb/test/API/commands/statistics/basic/TestStats.py
+++ lldb/test/API/commands/statistics/basic/TestStats.py
@@ -55,30 +55,6 @@
 self.assertEqual(success_fail_dict['failures'], num_fails,
  'make sure success count')
 
-def get_stats(self, options=None, log_path=None):
-"""
-Get the output of the "statistics dump" with optional extra options
-and return the JSON as a python dictionary.
-"""
-# If log_path is set, open the path and emit the output of the command
-# for debugging purposes.
-if log_path is not None:
-f = open(log_path, 'w')
-else:
-f = None
-return_obj = lldb.SBCommandReturnObject()
-command = "statistics dump "
-if options is not None:
-command += options
-if f:
-f.write('(lldb) %s\n' % (command))
-self.ci.HandleCommand(command, return_obj, False)
-metrics_json = return_obj.GetOutput()
-if f:
-f.write(metrics_json)
-return json.loads(metrics_json)
-
-
 def get_target_stats(self, debug_stats):
 if "targets" in debug_stats:
 return debug_stats["targets"][0]
@@ -509,7 +485,6 @@
 exe_name = 'a.out'
 exe = self.getBuildArtifact(exe_name)
 dsym = self.getBuildArtifact(exe_name + ".dSYM")
-print("carp: dsym = '%s'" % (dsym))
 # Make sure the executable file exists after building.
 self.assertEqual(os.path.exists(exe), True)
 # Make sure the dSYM file doesn't exist after building.
@@ -563,7 +538,6 @@
 exe = self.getBuildArtifact(exe_name)
 dsym = self.getBuildArtifact(exe_name + ".dSYM")
 main_obj = self.getBuildArtifact('main.o')
-print("carp: dsym = '%s'" % (dsym))
 # Make sure the executable file exists after building.
 self.assertEqual(os.path.exists(exe), True)
 # Make sure the dSYM file doesn't exist after building.
Index: lldb/source/Target/Statistics.cpp
===
--- lldb/source/Target/Statistics.cpp
+++ lldb/source/Target/Statistics.cpp
@@ -65,6 +65,8 @@
   module.try_emplace("debugInfoEnabled", debug_info_enabled);
   module.try_emplace("debugInfoHadVariableErrors",
  debug_info_had_variable_errors);
+  module.try_emplace("debugInfoHadIncompleteTypes",
+ debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
   if (!symfile_path.empty())
  

[Lldb-commits] [lldb] fc743f0 - Report which modules have forcefully completed types in statistics.

[Greg Clayton via lldb-commits]

Author: Greg Clayton
Date: 2022-11-30T21:22:27-08:00
New Revision: fc743f034a34d3fa0a6e4de3b34216e5ac5e4c93

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

LOG: Report which modules have forcefully completed types in statistics.

A previous patch added the ability for us to tell if types were forcefully 
completed. This patch adds the ability to see which modules have forcefully 
completed types and aggregates the number of modules with forcefully completed 
types at the root level.

We add a module specific setting named "debugInfoHadIncompleteTypes" that is a 
boolean value. We also aggregate the number of modules at the root level that 
had incomplete debug info with a key named 
"totalModuleCountWithIncompleteTypes" that is a count of number of modules that 
had incomplete types.

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

Added: 


Modified: 
lldb/include/lldb/Symbol/TypeSystem.h
lldb/include/lldb/Target/Statistics.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
lldb/source/Target/Statistics.cpp
lldb/test/API/commands/statistics/basic/TestStats.py
lldb/test/API/functionalities/limit-debug-info/TestLimitDebugInfo.py

Removed: 




diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h 
b/lldb/include/lldb/Symbol/TypeSystem.h
index c86a5215164ae..c9e727faba664 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -90,7 +90,7 @@ class TypeSystem : public PluginInterface,
   /// Free up any resources associated with this TypeSystem.  Done before
   /// removing all the TypeSystems from the TypeSystemMap.
   virtual void Finalize() {}
- 
+
   virtual DWARFASTParser *GetDWARFParser() { return nullptr; }
   virtual PDBASTParser *GetPDBParser() { return nullptr; }
   virtual npdb::PdbAstBuilder *GetNativePDBParser() { return nullptr; }
@@ -516,8 +516,13 @@ class TypeSystem : public PluginInterface,
 
   virtual llvm::Optional ReportStatistics();
 
+  bool GetHasForcefullyCompletedTypes() const {
+return m_has_forcefully_completed_types;
+  }
 protected:
   SymbolFile *m_sym_file = nullptr;
+  /// Used for reporting statistics.
+  bool m_has_forcefully_completed_types = false;
 };
 
 class TypeSystemMap {
@@ -541,6 +546,9 @@ class TypeSystemMap {
   GetTypeSystemForLanguage(lldb::LanguageType language, Target *target,
bool can_create);
 
+  /// Check all type systems in the map to see if any have forcefully completed
+  /// types;
+  bool GetHasForcefullyCompletedTypes() const;
 protected:
   typedef llvm::DenseMap collection;
   mutable std::mutex m_mutex; ///< A mutex to keep this object happy in

diff  --git a/lldb/include/lldb/Target/Statistics.h 
b/lldb/include/lldb/Target/Statistics.h
index 4bf2f3a69c9b1..485de9feea453 100644
--- a/lldb/include/lldb/Target/Statistics.h
+++ b/lldb/include/lldb/Target/Statistics.h
@@ -121,6 +121,7 @@ struct ModuleStats {
   bool debug_info_enabled = true;
   bool symtab_stripped = false;
   bool debug_info_had_variable_errors = false;
+  bool debug_info_had_incomplete_types = false;
 };
 
 struct ConstStringStats {

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 63bad9d0241de..d0501ef6b9d10 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -37,6 +37,7 @@
 import gc
 import glob
 import io
+import json
 import os.path
 import re
 import shutil
@@ -1642,6 +1643,19 @@ def run_platform_command(self, cmd):
 err = platform.Run(shell_command)
 return (err, shell_command.GetStatus(), shell_command.GetOutput())
 
+def get_stats(self, options=None):
+"""
+Get the output of the "statistics dump" with optional extra options
+and return the JSON as a python dictionary.
+"""
+return_obj = lldb.SBCommandReturnObject()
+command = "statistics dump "
+if options is not None:
+command += options
+self.ci.HandleCommand(command, return_obj, False)
+metrics_json = return_obj.GetOutput()
+return json.loads(metrics_json)
+
 # Metaclass for TestBase to change the list of test metods when a new TestCase 
is loaded.
 # We change the test methods to create a new test method for each test for 
each debug info we are
 # testing. The name of the new test method will be 
'_' and with adding
@@ -2483,7 +2497,7 @@ def assertCommandReturn(self, obj, msg=None):
 error = obj.GetError()
 

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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D138618#3948707 , @labath wrote:

> I am puzzled by the OSO changes in the DIERef class. How do they tie in with 
> the increase in the offset size? It seems like it should at best be a 
> separate patch...

I have been helping Alexander get this patch ready for open source. We needed 
to do these changes or this patch doesn't work and would break mac debugging. 
The reason is some people were manually creating lldb::user_id_t IDs and then 
manually decoding them. If we change the DIE offset size, then the people that 
were manually creating user_id_t would now be encoding bits into the wrong bits 
if they were every put into a DIERef we would extract the wrong information.

Part of what this patch is doing is allowing a DIERef to get a user_id_t from 
the object and also create itself from a user_id_t. This allows a single 
consistent interface. No one should be manually encoding user_id_t values now, 
and it should always be done through 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] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

[Adrian Prantl via Phabricator via lldb-commits]

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

I *think* this is what the code intended, yes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139066

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

[Lldb-commits] [PATCH] D139066: [lldb] Make sure the value of `eSymbolContextVariable` is not conflicting with `RESOLVED_FRAME_CODE_ADDR`

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139066

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


Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -44,7 +44,7 @@
 // The first bits in the flags are reserved for the SymbolContext::Scope bits
 // so we know if we have tried to look up information in our internal symbol
 // context (m_sc) already.
-#define RESOLVED_FRAME_CODE_ADDR (uint32_t(eSymbolContextEverything + 1))
+#define RESOLVED_FRAME_CODE_ADDR (uint32_t(eSymbolContextLastItem) << 1)
 #define RESOLVED_FRAME_ID_SYMBOL_SCOPE (RESOLVED_FRAME_CODE_ADDR << 1)
 #define GOT_FRAME_BASE (RESOLVED_FRAME_ID_SYMBOL_SCOPE << 1)
 #define RESOLVED_VARIABLES (GOT_FRAME_BASE << 1)
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -375,6 +375,9 @@
 /// from being used during frame PC lookups and many other
 /// potential address to symbol context lookups.
 eSymbolContextVariable = (1u << 7),
+
+// Keep this last and up-to-date for what the last enum value is.
+eSymbolContextLastItem = eSymbolContextVariable,
 };
 LLDB_MARK_AS_BITMASK_ENUM(SymbolContextItem)
 


Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -44,7 +44,7 @@
 // The first bits in the flags are reserved for the SymbolContext::Scope bits
 // so we know if we have tried to look up information in our internal symbol
 // context (m_sc) already.
-#define RESOLVED_FRAME_CODE_ADDR (uint32_t(eSymbolContextEverything + 1))
+#define RESOLVED_FRAME_CODE_ADDR (uint32_t(eSymbolContextLastItem) << 1)
 #define RESOLVED_FRAME_ID_SYMBOL_SCOPE (RESOLVED_FRAME_CODE_ADDR << 1)
 #define GOT_FRAME_BASE (RESOLVED_FRAME_ID_SYMBOL_SCOPE << 1)
 #define RESOLVED_VARIABLES (GOT_FRAME_BASE << 1)
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -375,6 +375,9 @@
 /// from being used during frame PC lookups and many other
 /// potential address to symbol context lookups.
 eSymbolContextVariable = (1u << 7),
+
+// Keep this last and up-to-date for what the last enum value is.
+eSymbolContextLastItem = eSymbolContextVariable,
 };
 LLDB_MARK_AS_BITMASK_ENUM(SymbolContextItem)
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138878: [lldb] Remove timer from Module::GetNumCompileUnits

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138878

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I think I'm fine with this variant modulo outstanding comments!




Comment at: lldb/source/Core/ModuleList.cpp:1080
+  bool ret = true;
+  ForEach([&](const ModuleSP _sp) {
+ret &= callback(module_sp);

kastiglione wrote:
> I wonder why ForEach doesn't deal out a `Module &`? I would think a 
> ModuleList should not allow for null Module pointers.
I think this is historic. +1 for taking a Module & (unless we for some reason 
need a shared_ptr in the lambda).



Comment at: lldb/source/Target/Target.cpp:1704
+
+// If a module was torn down it will have torn
+// down the 'TypeSystem's that we used as source

kastiglione wrote:
> nit: Why such a narrow line wrapping width?
Can you make it clear that this comment is talking about TypeSystemClang 
specifically?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D139058: [lldb/unittests/CMakeLists.txt] Remove extra compiler flag `-include gtest_common.h`, NFC

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Cool


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139058

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

[Lldb-commits] [PATCH] D139061: [lldb] Fix the `dwarf` log descriptions

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM




Comment at: lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp:15
 {{"comp"},
- {"log insertions of object files into DWARF debug maps"},
+ {"log struct/unions/class type completions"},
  DWARFLog::TypeCompletion},




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139061

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

[Lldb-commits] [PATCH] D139054: Delay calling ObjC class list read utility functions very early in process startup

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:2519-2520
+  case SharedCacheWarningReason::eExpressionUnableToRun:
+Debugger::ReportWarning("Objective-C class names could not be read "
+"right now, will retry later.\n",
+debugger.GetID(), nullptr);

[begin bikeshedding]
Could we be more descriptive? Right now this sounds unactionable, which to be 
fair, it kind of is. 

How about "could not execute support code to read Objective-C class data 
because it's not yet safe to do so and will be retried later.
[end bikeshedding]

Regardless we should say "class data" to match the other warnings or change 
those to say "class names". 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139054

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

[Lldb-commits] [PATCH] D139061: [lldb] Fix the `dwarf` log descriptions

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139061

Files:
  lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
@@ -12,14 +12,16 @@
 
 static constexpr Log::Category g_categories[] = {
 {{"comp"},
- {"log insertions of object files into DWARF debug maps"},
+ {"log struct/unions/class type completions"},
  DWARFLog::TypeCompletion},
 {{"info"}, {"log the parsing of .debug_info"}, DWARFLog::DebugInfo},
 {{"line"}, {"log the parsing of .debug_line"}, DWARFLog::DebugLine},
 {{"lookups"},
  {"log any lookups that happen by name, regex, or address"},
  DWARFLog::Lookups},
-{{"map"}, {"log struct/unions/class type completions"}, 
DWARFLog::DebugMap},
+{{"map"},
+ {"log insertions of object files into DWARF debug maps"},
+ DWARFLog::DebugMap},
 };
 
 static Log::Channel g_channel(g_categories, DWARFLog::DebugInfo);


Index: lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/LogChannelDWARF.cpp
@@ -12,14 +12,16 @@
 
 static constexpr Log::Category g_categories[] = {
 {{"comp"},
- {"log insertions of object files into DWARF debug maps"},
+ {"log struct/unions/class type completions"},
  DWARFLog::TypeCompletion},
 {{"info"}, {"log the parsing of .debug_info"}, DWARFLog::DebugInfo},
 {{"line"}, {"log the parsing of .debug_line"}, DWARFLog::DebugLine},
 {{"lookups"},
  {"log any lookups that happen by name, regex, or address"},
  DWARFLog::Lookups},
-{{"map"}, {"log struct/unions/class type completions"}, DWARFLog::DebugMap},
+{{"map"},
+ {"log insertions of object files into DWARF debug maps"},
+ DWARFLog::DebugMap},
 };
 
 static Log::Channel g_channel(g_categories, DWARFLog::DebugInfo);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM




Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:10
+#include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "Utility/UuidCompatibility.h"
 #include "lldb/Host/FileSystem.h"





Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:342-344
+  Log *log = GetLog(LLDBLog::Host);
+  LLDB_LOGF(log, "Error while searching for Xcode SDK: %s",
+toString(sdk_path_or_err.takeError()).c_str());




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

https://reviews.llvm.org/D138060

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

[Lldb-commits] [PATCH] D139058: [lldb/unittests/CMakeLists.txt] Remove extra compiler flag `-include gtest_common.h`, NFC

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

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

This doesn't seem to be necessary anymore so remove it to be more consistent 
with rest of the LLVM projects
that don't use prefix headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139058

Files:
  lldb/unittests/CMakeLists.txt


Index: lldb/unittests/CMakeLists.txt
===
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -10,13 +10,6 @@
   add_compile_options("-Wno-suggest-override")
 endif()
 
-set(LLDB_GTEST_COMMON_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/gtest_common.h)
-if (MSVC)
-  list(APPEND LLVM_COMPILE_FLAGS /FI ${LLDB_GTEST_COMMON_INCLUDE})
-else ()
-  list(APPEND LLVM_COMPILE_FLAGS -include ${LLDB_GTEST_COMMON_INCLUDE})
-endif ()
-
 function(add_lldb_unittest test_name)
   cmake_parse_arguments(ARG
 ""


Index: lldb/unittests/CMakeLists.txt
===
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -10,13 +10,6 @@
   add_compile_options("-Wno-suggest-override")
 endif()
 
-set(LLDB_GTEST_COMMON_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/gtest_common.h)
-if (MSVC)
-  list(APPEND LLVM_COMPILE_FLAGS /FI ${LLDB_GTEST_COMMON_INCLUDE})
-else ()
-  list(APPEND LLVM_COMPILE_FLAGS -include ${LLDB_GTEST_COMMON_INCLUDE})
-endif ()
-
 function(add_lldb_unittest test_name)
   cmake_parse_arguments(ARG
 ""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139054: Delay calling ObjC class list read utility functions very early in process startup

[Jason Molenda via Phabricator via lldb-commits]

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

When you are very early in process startup, before the system libraries have 
been initialized, and you run a simple expression, on Darwin systems, the 
Objective-C runtime plugin will run two utility functions to fetch the list of 
Objective-C class names in the inferior process.  These function calls can 
cause problems when the process launch is this early, and the user expression 
may have been a simple one like "p globalvar=1" which would be harmless and not 
require a jitted expression.

This patch adds code to debugserver to use libdyld calls to find the process 
launch state, returns it in a JSON reply for a "jGetDyldProcessState" packet.  
This is passed up to DynamicLoaderMacOS::IsFullyInitialized() which checks for 
three specific process states that happen early in process startup, before 
system library initialization is completed.

Thread::SafeToCallFunctions() currently, on macOS, checks to see if the current 
thread is in __select, and does not run utility functions if it is.  This patch 
updates SafeToCallFunctions() to also call DynamicLoader::IsFullyInitialized(), 
and avoid running utility functions if that is the case.

Then there are updates to AppleObjCRuntimeV2 to check the threads for 
SafeToCallFunctions() before scanning for dynamic objective-c classes, or the 
static shared cache objective-c classes.

I added a test case which stops process launch on the first malloc() call, runs 
a simple expression, and confirms (via the types log) that we did not read the 
objc class lists.  Then it continues to main(), runs the simple expression 
again, and confirms (via types log) that it did read the objc class list.

The patch looks a bit big, mostly from piping the data from 
MachProcess::GetDyldProcessState() up to 
DynamicLoaderMacOS::IsFullyInitialized() through a handful of layers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139054

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/Process.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.h
  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/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Thread.cpp
  lldb/test/API/macosx/early-process-launch/Makefile
  lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
  lldb/test/API/macosx/early-process-launch/main.c
  lldb/tools/debugserver/source/DNB.cpp
  lldb/tools/debugserver/source/DNB.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.h
  lldb/tools/debugserver/source/MacOSX/MachProcess.mm
  lldb/tools/debugserver/source/RNBRemote.cpp
  lldb/tools/debugserver/source/RNBRemote.h

Index: lldb/tools/debugserver/source/RNBRemote.h
===
--- lldb/tools/debugserver/source/RNBRemote.h
+++ lldb/tools/debugserver/source/RNBRemote.h
@@ -108,34 +108,35 @@
 json_query_get_shared_cache_info,  // 'jGetSharedCacheInfo'
 pass_signals_to_inferior,  // 'QPassSignals'
 start_noack_mode,  // 'QStartNoAckMode'
-prefix_reg_packets_with_tid,// 'QPrefixRegisterPacketsWithThreadID
-set_logging_mode,   // 'QSetLogging:'
-set_ignored_exceptions, // 'QSetIgnoredExceptions'   
-set_max_packet_size,// 'QSetMaxPacketSize:'
-set_max_payload_size,   // 'QSetMaxPayloadSize:'
-set_environment_variable,   // 'QEnvironment:'
-set_environment_variable_hex,   // 'QEnvironmentHexEncoded:'
-set_launch_arch,// 'QLaunchArch:'
-set_disable_aslr,   // 'QSetDisableASLR:'
-set_stdin,  // 'QSetSTDIN:'
-set_stdout, // 'QSetSTDOUT:'
-set_stderr, // 'QSetSTDERR:'
-set_working_dir,// 'QSetWorkingDir:'
-set_list_threads_in_stop_reply, // 'QListThreadsInStopReply:'
-sync_thread_state,  // 'QSyncThreadState:'
-memory_region_info, // 'qMemoryRegionInfo:'
-get_profile_data,   // 'qGetProfileData'
-set_enable_profiling,   // 'QSetEnableAsyncProfiling'
-enable_compression, // 

[Lldb-commits] [lldb] 7a63907 - Add a log message to the IR interpreter.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-11-30T14:08:14-08:00
New Revision: 7a63907021344b3aed322cd86bfa09678653e5dc

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

LOG: Add a log message to the IR interpreter.

This line existed in our internal branch and seems to be generally
useful, so I'm upstreaming it.

Added: 


Modified: 
lldb/source/Expression/IRInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Expression/IRInterpreter.cpp 
b/lldb/source/Expression/IRInterpreter.cpp
index d03fea6ffce3..8e09e9f02244 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -531,6 +531,7 @@ bool IRInterpreter::CanInterpret(llvm::Module , 
llvm::Function ,
 return false;
   }
   saw_function_with_body = true;
+  LLDB_LOGF(log, "Saw function with body: %s", f.getName().str().c_str());
 }
   }
 



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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/source/Core/ModuleList.cpp:1080
+  bool ret = true;
+  ForEach([&](const ModuleSP _sp) {
+ret &= callback(module_sp);

I wonder why ForEach doesn't deal out a `Module &`? I would think a ModuleList 
should not allow for null Module pointers.



Comment at: lldb/source/Target/Target.cpp:1686
+const bool should_flush_type_systems =
+module_list.AllOf([](const lldb::ModuleSP _sp) {
+  if (!module_sp)

How come this is `AllOf` and not a `AnyOf`?



Comment at: lldb/source/Target/Target.cpp:1704-1710
+// If a module was torn down it will have torn
+// down the 'TypeSystem's that we used as source
+// 'ASTContext's for the persistent variables
+// in the current target. Those would now be
+// unsafe to access because the 'DeclOrigin'
+// are now possibly stale. Thus clear all
+// persistent variables.

nit: Why such a narrow line wrapping width?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Arthur Eubanks via Phabricator via lldb-commits]

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

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138834

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/API/lang/cpp/unique-types3/Makefile
  lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
  lldb/test/API/lang/cpp/unique-types3/a.cpp
  lldb/test/API/lang/cpp/unique-types3/a.h
  lldb/test/API/lang/cpp/unique-types3/main.cpp

Index: lldb/test/API/lang/cpp/unique-types3/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/main.cpp
@@ -0,0 +1,9 @@
+#include "a.h"
+
+S a1;
+S a2;
+S a3;
+
+void f(S &);
+
+int main() { f(a2); }
Index: lldb/test/API/lang/cpp/unique-types3/a.h
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/a.h
@@ -0,0 +1,3 @@
+template  struct S {
+  T t;
+};
Index: lldb/test/API/lang/cpp/unique-types3/a.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/a.cpp
@@ -0,0 +1,5 @@
+#include "a.h"
+
+void f(S ) {
+  (void)a; // Set breakpoint here
+}
Index: lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/TestUniqueTypes3.py
@@ -0,0 +1,27 @@
+"""
+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 UniqueTypesTestCase3(TestBase):
+def do_test(self, debug_flags):
+"""Test that we display the correct template instantiation."""
+self.build(dictionary=debug_flags)
+lldbutil.run_to_source_breakpoint(self, "// Set breakpoint here", lldb.SBFileSpec("a.cpp"))
+self.expect_expr("a", result_type="S")
+
+@skipIf(compiler=no_match("clang"))
+@skipIf(compiler_version=["<", "15.0"])
+def test_simple_template_names(self):
+# Can't directly set CFLAGS_EXTRAS here because the Makefile can't
+# override an environment variable.
+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(CFLAGS_EXTRAS="-gno-simple-template-names"))
Index: lldb/test/API/lang/cpp/unique-types3/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/unique-types3/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp a.cpp
+
+CFLAGS_EXTRAS = $(TEST_CFLAGS_EXTRAS) $(LIMIT_DEBUG_INFO_FLAGS)
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2978,6 +2978,16 @@
   }
 }
 
+// See comments below about -gsimple-template-names for why we attempt to
+// compute missing template parameter names.
+ConstString template_params;
+if (type_system && !llvm::StringRef(die.GetName()).contains('<')) {
+  DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
+  if (dwarf_ast)
+template_params =
+dwarf_ast->GetForwardDeclarationDIETemplateParams(die);
+}
+
 m_index->GetTypes(GetDWARFDeclContext(die), [&](DWARFDIE type_die) {
   // Make sure type_die's language matches the type system we are
   // looking for. We don't want to find a "Foo" type from Java if we
@@ -3049,6 +3059,27 @@
   if (!resolved_type || resolved_type == DIE_IS_BEING_PARSED)
 return true;
 
+  // With -gsimple-template-names, the DIE name may not contain the template
+  // parameters. If we've the declaration has template parameters but
+  // doesn't contain '<', check that the child template parameters match.
+  if (template_params) {
+llvm::StringRef test_base_name =
+GetTypeForDIE(type_die)->GetBaseName().GetStringRef();
+auto i = test_base_name.find('<');
+
+// Full name from clang AST doesn't contain '<' so this type_die isn't
+// a template parameter, but we're expecting template parameters, so
+// bail.
+if (i == llvm::StringRef::npos)
+  return true;
+
+llvm::StringRef test_template_params =
+test_base_name.slice(i, test_base_name.size());
+// Bail if template parameters don't 

[Lldb-commits] [PATCH] D138612: [lldb] Change FindDefinitionTypeForDWARFDeclContext() to take DWARFDIE

[Arthur Eubanks 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 rG61aed52c9ec0: [lldb] Change 
FindDefinitionTypeForDWARFDeclContext() to take DWARFDIE (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138612

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -63,8 +63,8 @@
 
   UniqueDWARFASTTypeMap () override;
 
-  lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(
-  const DWARFDeclContext _decl_ctx) override;
+  lldb::TypeSP
+  FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) override;
 
   lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
   const DWARFDIE , lldb_private::ConstString type_name,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -117,10 +117,9 @@
   return GetBaseSymbolFile().GetUniqueDWARFASTTypeMap();
 }
 
-lldb::TypeSP SymbolFileDWARFDwo::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
-  return GetBaseSymbolFile().FindDefinitionTypeForDWARFDeclContext(
-  die_decl_ctx);
+lldb::TypeSP
+SymbolFileDWARFDwo::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
+  return GetBaseSymbolFile().FindDefinitionTypeForDWARFDeclContext(die);
 }
 
 lldb::TypeSP SymbolFileDWARFDwo::FindCompleteObjCDefinitionTypeForDIE(
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -282,8 +282,7 @@
 
   CompileUnitInfo *GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf);
 
-  lldb::TypeSP
-  FindDefinitionTypeForDWARFDeclContext(const DWARFDeclContext _decl_ctx);
+  lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(const DWARFDIE );
 
   bool Supports_DW_AT_APPLE_objc_complete_type(SymbolFileDWARF *skip_dwarf_oso);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1125,10 +1125,10 @@
 }
 
 TypeSP SymbolFileDWARFDebugMap::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
+const DWARFDIE ) {
   TypeSP type_sp;
   ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
-type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die_decl_ctx);
+type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die);
 return ((bool)type_sp);
   });
   return type_sp;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -443,7 +443,7 @@
lldb_private::SymbolContext );
 
   virtual lldb::TypeSP
-  FindDefinitionTypeForDWARFDeclContext(const DWARFDeclContext _decl_ctx);
+  FindDefinitionTypeForDWARFDeclContext(const DWARFDIE );
 
   virtual lldb::TypeSP
   FindCompleteObjCDefinitionTypeForDIE(const DWARFDIE ,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2946,119 +2946,112 @@
   return true;
 }
 
-TypeSP SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
+TypeSP
+SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
   TypeSP type_sp;
 
-  const uint32_t dwarf_decl_ctx_count = dwarf_decl_ctx.GetSize();
-  if (dwarf_decl_ctx_count > 0) {
-const ConstString type_name(dwarf_decl_ctx[0].name);
-const dw_tag_t tag = dwarf_decl_ctx[0].tag;
+  if (die.GetName()) {
+const 

[Lldb-commits] [lldb] 61aed52 - [lldb] Change FindDefinitionTypeForDWARFDeclContext() to take DWARFDIE

[Arthur Eubanks via lldb-commits]

Author: Arthur Eubanks
Date: 2022-11-30T13:20:23-08:00
New Revision: 61aed52c9ec0bca9979922ed2762785ec1f39755

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

LOG: [lldb] Change FindDefinitionTypeForDWARFDeclContext() to take DWARFDIE

This simplifies an upcoming patch.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a340f1d3d8fe6..2a71705ec064f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -753,17 +753,14 @@ TypeSP DWARFASTParserClang::ParseEnum(const SymbolContext 
,
 if (type_sp)
   return type_sp;
 
-DWARFDeclContext die_decl_ctx = SymbolFileDWARF::GetDWARFDeclContext(die);
-
-type_sp = dwarf->FindDefinitionTypeForDWARFDeclContext(die_decl_ctx);
+type_sp = dwarf->FindDefinitionTypeForDWARFDeclContext(die);
 
 if (!type_sp) {
   SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
   if (debug_map_symfile) {
 // We weren't able to find a full declaration in this DWARF,
 // see if we have a declaration anywhere else...
-type_sp = debug_map_symfile->FindDefinitionTypeForDWARFDeclContext(
-die_decl_ctx);
+type_sp = 
debug_map_symfile->FindDefinitionTypeForDWARFDeclContext(die);
   }
 }
 
@@ -1732,19 +1729,16 @@ DWARFASTParserClang::ParseStructureLikeDIE(const 
SymbolContext ,
 if (type_sp)
   return type_sp;
 
-DWARFDeclContext die_decl_ctx = SymbolFileDWARF::GetDWARFDeclContext(die);
-
 // type_sp = FindDefinitionTypeForDIE (dwarf_cu, die,
 // type_name_const_str);
-type_sp = dwarf->FindDefinitionTypeForDWARFDeclContext(die_decl_ctx);
+type_sp = dwarf->FindDefinitionTypeForDWARFDeclContext(die);
 
 if (!type_sp) {
   SymbolFileDWARFDebugMap *debug_map_symfile = dwarf->GetDebugMapSymfile();
   if (debug_map_symfile) {
 // We weren't able to find a full declaration in this DWARF, see
 // if we have a declaration anywhere else...
-type_sp = debug_map_symfile->FindDefinitionTypeForDWARFDeclContext(
-die_decl_ctx);
+type_sp = 
debug_map_symfile->FindDefinitionTypeForDWARFDeclContext(die);
   }
 }
 

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
index e46694405415b..fa776f27d5019 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
@@ -75,15 +75,10 @@ class DWARFDeclContext {
 m_qualified_name.clear();
   }
 
-  lldb::LanguageType GetLanguage() const { return m_language; }
-
-  void SetLanguage(lldb::LanguageType language) { m_language = language; }
-
 protected:
   typedef std::vector collection;
   collection m_entries;
   mutable std::string m_qualified_name;
-  lldb::LanguageType m_language = lldb::eLanguageTypeUnknown;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDECLCONTEXT_H

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index fef2a36384703..bd29d4120e9b1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2946,119 +2946,112 @@ bool SymbolFileDWARF::DIEDeclContextsMatch(const 
DWARFDIE ,
   return true;
 }
 
-TypeSP SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
+TypeSP
+SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
   TypeSP type_sp;
 
-  const uint32_t dwarf_decl_ctx_count = dwarf_decl_ctx.GetSize();
-  if (dwarf_decl_ctx_count > 0) {
-const ConstString type_name(dwarf_decl_ctx[0].name);
-const dw_tag_t tag = dwarf_decl_ctx[0].tag;
+  if (die.GetName()) {
+const dw_tag_t tag = die.Tag();
 
-if (type_name) {
-  Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
-  if (log) {
-GetObjectFile()->GetModule()->LogMessage(
- 

[Lldb-commits] [PATCH] D138892: [DataFormatter] Fix variant npos with `_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` enabled.

[Jordan Rupprecht via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c51ea3619e4: [DataFormatter] Fix variant npos with 
`_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZAT… (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138892

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
@@ -29,6 +29,34 @@
 std::variant v3;
 std::variant> v_v1 ;
 std::variant v_no_value;
+// The next variant has 300 types, meaning the type index does not fit in
+// a byte and must be `unsigned short` instead of `unsigned char` when
+// using the unstable libc++ ABI. With stable libc++ ABI, the type index
+// is always just `unsigned int`.
+std::variant<
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+int, int, int, int, int, int>
+v_300_types_no_value;
 
 v1 = 12; // v contains int
 v_v1 = v1 ;
@@ -54,6 +82,13 @@
  } catch( ... ) {}
 
  printf( "%zu\n", v_no_value.index() ) ;
+
+ try {
+   v_300_types_no_value.emplace<0>(S());
+ } catch (...) {
+ }
+
+ printf("%zu\n", v_300_types_no_value.index());
 #endif
 
 return 0; // break here
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
@@ -76,3 +76,6 @@
 
 self.expect("frame variable v_no_value",
 substrs=['v_no_value =  No Value'])
+
+self.expect("frame variable v_300_types_no_value",
+substrs=['v_300_types_no_value =  No Value'])
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -9,6 +9,8 @@
 #include "LibCxxVariant.h"
 #include "LibCxx.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Symbol/CompilerType.h"
+#include "lldb/Utility/LLDBAssert.h"
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -66,6 +68,19 @@
 // 3) NPos, its value is variant_npos which means the variant has no value
 enum class LibcxxVariantIndexValidity { Valid, Invalid, NPos };
 
+uint64_t VariantNposValue(uint64_t index_byte_size) {
+  switch (index_byte_size) {
+  case 1:
+return static_cast(-1);
+  case 2:
+return static_cast(-1);
+  case 4:
+return static_cast(-1);
+  }
+  

[Lldb-commits] [lldb] 3c51ea3 - [DataFormatter] Fix variant npos with `_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` enabled.

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2022-11-30T13:20:13-08:00
New Revision: 3c51ea3619e488db19cd26840ed46d58cfc7062f

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

LOG: [DataFormatter] Fix variant npos with 
`_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` enabled.

This data formatter should print "No Value" if a variant is unset. It does so 
by checking if `__index` has a value of `-1`, however it does so by 
interpreting it as a signed int.

By default, `__index` has type `unsigned int`. When 
`_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` is enabled, the type of `__index` 
is either `unsigned char`, `unsigned short`, or `unsigned int`, depending on 
how many fields there are -- as small as possible. For example, when 
`std::variant` has only a few types, the index type is `unsigned char`, and the 
npos value will be interpreted by LLDB as `255` when it should be `-1`.

This change does not special case the variant optimization; it just reads the 
type instead of assuming it's `unsigned int`.

Reviewed By: labath

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

Added: 


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

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
index 8f90c56890e43..464425e291a6d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp
@@ -9,6 +9,8 @@
 #include "LibCxxVariant.h"
 #include "LibCxx.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Symbol/CompilerType.h"
+#include "lldb/Utility/LLDBAssert.h"
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -66,6 +68,19 @@ namespace {
 // 3) NPos, its value is variant_npos which means the variant has no value
 enum class LibcxxVariantIndexValidity { Valid, Invalid, NPos };
 
+uint64_t VariantNposValue(uint64_t index_byte_size) {
+  switch (index_byte_size) {
+  case 1:
+return static_cast(-1);
+  case 2:
+return static_cast(-1);
+  case 4:
+return static_cast(-1);
+  }
+  lldbassert(false && "Unknown index type size");
+  return static_cast(-1); // Fallback to stable ABI type.
+}
+
 LibcxxVariantIndexValidity
 LibcxxVariantGetIndexValidity(ValueObjectSP _sp) {
   ValueObjectSP index_sp(
@@ -74,9 +89,23 @@ LibcxxVariantGetIndexValidity(ValueObjectSP _sp) {
   if (!index_sp)
 return LibcxxVariantIndexValidity::Invalid;
 
-  int64_t index_value = index_sp->GetValueAsSigned(0);
+  // In the stable ABI, the type of __index is just int.
+  // In the unstable ABI, where _LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION is
+  // enabled, the type can either be unsigned char/short/int depending on
+  // how many variant types there are.
+  // We only need to do this here when comparing against npos, because npos is
+  // just `-1`, but that translates to 
diff erent unsigned values depending on
+  // the byte size.
+  CompilerType index_type = index_sp->GetCompilerType();
+
+  llvm::Optional index_type_bytes = index_type.GetByteSize(nullptr);
+  if (!index_type_bytes)
+return LibcxxVariantIndexValidity::Invalid;
+
+  uint64_t npos_value = VariantNposValue(*index_type_bytes);
+  uint64_t index_value = index_sp->GetValueAsUnsigned(0);
 
-  if (index_value == -1)
+  if (index_value == npos_value)
 return LibcxxVariantIndexValidity::NPos;
 
   return LibcxxVariantIndexValidity::Valid;

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
index fbd7c8b21b9e4..a2bb31260a85b 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
@@ -76,3 +76,6 @@ def test_with_run_command(self):
 
 self.expect("frame variable v_no_value",
 substrs=['v_no_value =  No Value'])
+
+self.expect("frame variable v_300_types_no_value",
+substrs=['v_300_types_no_value =  No Value'])

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
index c0bc4ae12c1ae..560ec692f30ed 100644
--- 

[Lldb-commits] [PATCH] D138612: [lldb] Change FindDefinitionTypeForDWARFDeclContext() to take DWARFDIE

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added a comment.

I borrowed a mac and verified that this patch fixes the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138612

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

[Lldb-commits] [PATCH] D138612: [lldb] Change FindDefinitionTypeForDWARFDeclContext() to take DWARFDIE

[Arthur Eubanks via Phabricator via lldb-commits]

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

go back to calling m_index->GetTypes(DWARFDeclContext) instead of 
m_index->GetTypes(StringRef)

these have different codepaths, I mistakenly thought they were equivalent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138612

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDeclContext.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -63,8 +63,8 @@
 
   UniqueDWARFASTTypeMap () override;
 
-  lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(
-  const DWARFDeclContext _decl_ctx) override;
+  lldb::TypeSP
+  FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) override;
 
   lldb::TypeSP FindCompleteObjCDefinitionTypeForDIE(
   const DWARFDIE , lldb_private::ConstString type_name,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -117,10 +117,9 @@
   return GetBaseSymbolFile().GetUniqueDWARFASTTypeMap();
 }
 
-lldb::TypeSP SymbolFileDWARFDwo::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
-  return GetBaseSymbolFile().FindDefinitionTypeForDWARFDeclContext(
-  die_decl_ctx);
+lldb::TypeSP
+SymbolFileDWARFDwo::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
+  return GetBaseSymbolFile().FindDefinitionTypeForDWARFDeclContext(die);
 }
 
 lldb::TypeSP SymbolFileDWARFDwo::FindCompleteObjCDefinitionTypeForDIE(
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -282,8 +282,7 @@
 
   CompileUnitInfo *GetCompileUnitInfo(SymbolFileDWARF *oso_dwarf);
 
-  lldb::TypeSP
-  FindDefinitionTypeForDWARFDeclContext(const DWARFDeclContext _decl_ctx);
+  lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(const DWARFDIE );
 
   bool Supports_DW_AT_APPLE_objc_complete_type(SymbolFileDWARF *skip_dwarf_oso);
 
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1125,10 +1125,10 @@
 }
 
 TypeSP SymbolFileDWARFDebugMap::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
+const DWARFDIE ) {
   TypeSP type_sp;
   ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
-type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die_decl_ctx);
+type_sp = oso_dwarf->FindDefinitionTypeForDWARFDeclContext(die);
 return ((bool)type_sp);
   });
   return type_sp;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -443,7 +443,7 @@
lldb_private::SymbolContext );
 
   virtual lldb::TypeSP
-  FindDefinitionTypeForDWARFDeclContext(const DWARFDeclContext _decl_ctx);
+  FindDefinitionTypeForDWARFDeclContext(const DWARFDIE );
 
   virtual lldb::TypeSP
   FindCompleteObjCDefinitionTypeForDIE(const DWARFDIE ,
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2946,119 +2946,112 @@
   return true;
 }
 
-TypeSP SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(
-const DWARFDeclContext _decl_ctx) {
+TypeSP
+SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE ) {
   TypeSP type_sp;
 
-  const uint32_t dwarf_decl_ctx_count = dwarf_decl_ctx.GetSize();
-  if (dwarf_decl_ctx_count > 0) {
-const ConstString type_name(dwarf_decl_ctx[0].name);
-const dw_tag_t tag = dwarf_decl_ctx[0].tag;
+  if (die.GetName()) {
+const dw_tag_t tag = 

[Lldb-commits] [PATCH] D138892: [DataFormatter] Fix variant npos with `_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` enabled.

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

I plan to land this now so we can remove this from our internal test exclusion 
list, but I'd be happy to address any post-commit feedback from other reviewers 
as a followup change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138892

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Target/Target.cpp:208
+// of the debugee.
+m_scratch_type_system_map.Clear();
 m_process_sp.reset();

aprantl wrote:
> Michael137 wrote:
> > Michael137 wrote:
> > > kastiglione wrote:
> > > > Do we have some place in the life-cycle where we can perform this only 
> > > > if the target has changed? Ideally this would happen when the binary 
> > > > has a different timestamp, or for mach-o a different UUID.
> > > There is `DidUnloadModules` which gets notified when an 
> > > lldb_private::Module gets unloaded (e.g., on rebuilt). But this includes 
> > > JITted modules (e.g., when running AppleObjectiveCRuntimeV2 utility 
> > > functions) in which case we wouldn’t want to flush the type systems. Also 
> > > the Clang REPL (and I assume the Swift REPL) rely on the type system 
> > > being still present after we unloaded the module associated with the 
> > > evaluated expression. All this is to say, I found it to be quite fiddly 
> > > to determine when to flush the persistent variables from within that 
> > > notification. Really we would like a notification to the Target which 
> > > says “we restarted the debugee AND some module got rebuilt”. In that case 
> > > it’s not safe to keep the persistent variables around. I’ll double check 
> > > if there isn’t something like that around.
> > I suppose we could do something like:
> > 
> > ```
> > void Target::ModulesDidUnload(ModuleList _list, bool 
> > delete_locations) {
> > ...
> > if each module in module_list is Type::eTypeExecutable or 
> > Type::eTypeObjectFile {
> > m_scratch_type_system_map.Clear();
> > }
> > ```
> That looks like a reasonable middle ground?
Updated diff with this alternative


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

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

- Clear TypeSystem in notification


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/rerun_and_expr/Makefile
  lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
  lldb/test/API/functionalities/rerun_and_expr/main.cpp
  lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp

Index: lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
@@ -0,0 +1,12 @@
+struct Base {
+  int m_base_val = 42;
+};
+
+struct Foo : public Base {
+  int m_derived_val = 137;
+};
+
+int main() {
+  Foo foo;
+  return 0;
+}
Index: lldb/test/API/functionalities/rerun_and_expr/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/main.cpp
@@ -0,0 +1,8 @@
+struct Foo {
+  int m_val = 42;
+};
+
+int main() {
+  Foo foo;
+  return 0;
+}
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -0,0 +1,71 @@
+"""
+Test that re-running a process from within the same target
+after rebuilding the executable flushes the scratch TypeSystems
+tied to that process.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestRerun(TestBase):
+def test(self):
+"""
+Tests whether re-launching a process without destroying
+the owning target keeps invalid ASTContexts in the
+scratch AST's importer.
+
+We test this by:
+1. Evaluating an expression to import 'struct Foo' into
+   the scratch AST
+2. Change the definition of 'struct Foo' and rebuild the executable
+3. Re-launch the process
+4. Evaluate the same expression in (1). We expect to have only
+   the latest definition of 'struct Foo' in the scratch AST.
+"""
+self.build(dictionary={'CXX_SOURCES':'main.cpp', 'EXE':'a.out'})
+(target, _, _, bkpt) = \
+lldbutil.run_to_source_breakpoint(self, 'return', lldb.SBFileSpec('main.cpp'))
+
+target.BreakpointCreateBySourceRegex('return', lldb.SBFileSpec('rebuild.cpp', False))
+
+self.expect_expr('foo', result_type='Foo', result_children=[
+ValueCheck(name='m_val', value='42')
+])
+
+self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
+
+self.runCmd('process launch')
+
+self.expect_expr('foo', result_type='Foo', result_children=[
+ValueCheck(name='Base', children=[
+ValueCheck(name='m_base_val', value='42')
+]),
+ValueCheck(name='m_derived_val', value='137')
+])
+
+self.filecheck("target module dump ast", __file__)
+
+# The new definition 'struct Foo' is in the scratch AST
+# CHECK:  |-CXXRecordDecl {{.*}} struct Foo definition
+# CHECK-NEXT: | |-DefinitionData pass_in_registers standard_layout trivially_copyable 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
+# CHECK-NEXT: | |-public 'Base'
+# CHECK-NEXT: | `-FieldDecl {{.*}} m_derived_val 'int'
+# CHECK-NEXT: `-CXXRecordDecl {{.*}} struct Base definition
+# 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
+
+# ...but the original definition of 'struct Foo' is not in the scratch 

[Lldb-commits] [lldb] b5467ec - [lldb] Use LLDB_ENABLE_SWIG as the canonical CMake variable

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2022-11-30T12:46:13-08:00
New Revision: b5467ecc0f3b22082023e99412d4148ac41530be

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

LOG: [lldb] Use LLDB_ENABLE_SWIG as the canonical CMake variable

Use LLDB_ENABLE_SWIG instead of SWIG_EXECUTABLE or SWIG_FOUND as the
canonical CMake variable to determine whether we have SWIG available in
LLDB. This is a follow-up to b3c978e850d3.

Added: 


Modified: 
lldb/cmake/modules/FindLuaAndSwig.cmake
lldb/cmake/modules/FindPythonAndSwig.cmake

Removed: 




diff  --git a/lldb/cmake/modules/FindLuaAndSwig.cmake 
b/lldb/cmake/modules/FindLuaAndSwig.cmake
index dd51ea1ef4c18..763bf0a7bb993 100644
--- a/lldb/cmake/modules/FindLuaAndSwig.cmake
+++ b/lldb/cmake/modules/FindLuaAndSwig.cmake
@@ -4,16 +4,15 @@
 #
 # Find Lua and SWIG as a whole.
 
-if(LUA_LIBRARIES AND LUA_INCLUDE_DIR AND SWIG_EXECUTABLE)
+if(LUA_LIBRARIES AND LUA_INCLUDE_DIR AND LLDB_ENABLE_SWIG)
   set(LUAANDSWIG_FOUND TRUE)
 else()
   if (LLDB_ENABLE_SWIG)
 find_package(Lua 5.3 EXACT)
-if(LUA_FOUND AND SWIG_FOUND)
+if(LUA_FOUND)
   mark_as_advanced(
 LUA_LIBRARIES
-LUA_INCLUDE_DIR
-SWIG_EXECUTABLE)
+LUA_INCLUDE_DIR)
 endif()
   else()
 message(STATUS "SWIG 3 or later is required for Lua support in LLDB but 
could not be found")

diff  --git a/lldb/cmake/modules/FindPythonAndSwig.cmake 
b/lldb/cmake/modules/FindPythonAndSwig.cmake
index 1b7e1657e364b..d9305ab31f225 100644
--- a/lldb/cmake/modules/FindPythonAndSwig.cmake
+++ b/lldb/cmake/modules/FindPythonAndSwig.cmake
@@ -30,12 +30,11 @@ macro(FindPython3)
   Python3_LIBRARIES
   Python3_INCLUDE_DIRS
   Python3_EXECUTABLE
-  Python3_RPATH
-  SWIG_EXECUTABLE)
+  Python3_RPATH)
   endif()
 endmacro()
 
-if(Python3_LIBRARIES AND Python3_INCLUDE_DIRS AND Python3_EXECUTABLE AND 
SWIG_EXECUTABLE)
+if(Python3_LIBRARIES AND Python3_INCLUDE_DIRS AND Python3_EXECUTABLE AND 
LLDB_ENABLE_SWIG)
   set(PYTHONANDSWIG_FOUND TRUE)
 else()
   if (LLDB_ENABLE_SWIG)



___
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.

ping


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] D138960: [lldb] Enable use of dummy target from dwim-print

[Dave Lee via Phabricator via lldb-commits]

kastiglione updated this revision to Diff 479035.
kastiglione added a comment.

Add docstring to new test function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138960

Files:
  lldb/source/Commands/CommandObjectDWIMPrint.cpp
  lldb/test/API/commands/dwim-print/TestDWIMPrint.py

Index: lldb/test/API/commands/dwim-print/TestDWIMPrint.py
===
--- lldb/test/API/commands/dwim-print/TestDWIMPrint.py
+++ lldb/test/API/commands/dwim-print/TestDWIMPrint.py
@@ -10,11 +10,6 @@
 
 
 class TestCase(TestBase):
-def setUp(self):
-TestBase.setUp(self)
-self.build()
-lldbutil.run_to_name_breakpoint(self, "main")
-
 def _run_cmd(self, cmd: str) -> str:
 """Run the given lldb command and return its output."""
 result = lldb.SBCommandReturnObject()
@@ -51,18 +46,28 @@
 
 def test_variables(self):
 """Test dwim-print with variables."""
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
 vars = ("argc", "argv")
 for var in vars:
 self._expect_cmd(var, "frame variable")
 
 def test_variable_paths(self):
 """Test dwim-print with variable path expressions."""
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("", "*argv", "argv[0]")
 for expr in exprs:
 self._expect_cmd(expr, "expression --")
 
 def test_expressions(self):
 """Test dwim-print with expressions."""
+self.build()
+lldbutil.run_to_name_breakpoint(self, "main")
 exprs = ("argc + 1", "(void)argc", "(int)abs(argc)")
 for expr in exprs:
 self._expect_cmd(expr, "expression --")
+
+def test_dummy_target_expressions(self):
+"""Test dwim-print's ability to evaluate expressions without a target."""
+self._expect_cmd("1 + 2", "expression --")
Index: lldb/source/Commands/CommandObjectDWIMPrint.cpp
===
--- lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -22,12 +22,11 @@
 using namespace lldb_private;
 
 CommandObjectDWIMPrint::CommandObjectDWIMPrint(CommandInterpreter )
-: CommandObjectRaw(
-  interpreter, "dwim-print", "Print a variable or expression.",
-  "dwim-print [ | ]",
-  eCommandProcessMustBePaused | eCommandTryTargetAPILock |
-  eCommandRequiresFrame | eCommandProcessMustBeLaunched |
-  eCommandRequiresProcess) {}
+: CommandObjectRaw(interpreter, "dwim-print",
+   "Print a variable or expression.",
+   "dwim-print [ | ]",
+   eCommandProcessMustBePaused | eCommandTryTargetAPILock) {
+}
 
 bool CommandObjectDWIMPrint::DoExecute(StringRef expr,
CommandReturnObject ) {
@@ -40,14 +39,10 @@
 return false;
   }
 
-  // eCommandRequiresFrame guarantees a frame.
-  StackFrame *frame = m_exe_ctx.GetFramePtr();
-  assert(frame);
-
   auto verbosity = GetDebugger().GetDWIMPrintVerbosity();
 
-  // First, try `expr` as the name of a variable.
-  {
+  // First, try `expr` as the name of a frame variable.
+  if (StackFrame *frame = m_exe_ctx.GetFramePtr()) {
 auto valobj_sp = frame->FindVariable(ConstString(expr));
 if (valobj_sp && valobj_sp->GetError().Success()) {
   if (verbosity == eDWIMPrintVerbosityFull)
@@ -60,12 +55,13 @@
 
   // Second, also lastly, try `expr` as a source expression to evaluate.
   {
-// eCommandRequiresProcess guarantees a target.
-Target *target = m_exe_ctx.GetTargetPtr();
-assert(target);
+Target *target_ptr = m_exe_ctx.GetTargetPtr();
+// Fallback to the dummy target, which can allow for expression evaluation.
+Target  = target_ptr ? *target_ptr : GetDummyTarget();
 
+auto *exe_scope = m_exe_ctx.GetBestExecutionContextScope();
 ValueObjectSP valobj_sp;
-if (target->EvaluateExpression(expr, frame, valobj_sp) ==
+if (target.EvaluateExpression(expr, exe_scope, valobj_sp) ==
 eExpressionCompleted) {
   if (verbosity != eDWIMPrintVerbosityNone)
 result.AppendMessageWithFormatv("note: ran `expression -- {0}`", expr);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp:36
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override {
+// Since we only have a single child, return it
+assert(idx == 0);

To be very nitpicking: LLVM style wants full sentences in comments, so let's 
add a `.` at the end. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138558

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/source/Target/Target.cpp:208
+// of the debugee.
+m_scratch_type_system_map.Clear();
 m_process_sp.reset();

Michael137 wrote:
> Michael137 wrote:
> > kastiglione wrote:
> > > Do we have some place in the life-cycle where we can perform this only if 
> > > the target has changed? Ideally this would happen when the binary has a 
> > > different timestamp, or for mach-o a different UUID.
> > There is `DidUnloadModules` which gets notified when an 
> > lldb_private::Module gets unloaded (e.g., on rebuilt). But this includes 
> > JITted modules (e.g., when running AppleObjectiveCRuntimeV2 utility 
> > functions) in which case we wouldn’t want to flush the type systems. Also 
> > the Clang REPL (and I assume the Swift REPL) rely on the type system being 
> > still present after we unloaded the module associated with the evaluated 
> > expression. All this is to say, I found it to be quite fiddly to determine 
> > when to flush the persistent variables from within that notification. 
> > Really we would like a notification to the Target which says “we restarted 
> > the debugee AND some module got rebuilt”. In that case it’s not safe to 
> > keep the persistent variables around. I’ll double check if there isn’t 
> > something like that around.
> I suppose we could do something like:
> 
> ```
> void Target::ModulesDidUnload(ModuleList _list, bool delete_locations) 
> {
> ...
> if each module in module_list is Type::eTypeExecutable or 
> Type::eTypeObjectFile {
> m_scratch_type_system_map.Clear();
> }
> ```
That looks like a reasonable middle ground?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D132735#3959530 , @avogelsgesang 
wrote:

> As discussed via email, the reason for the test failures on green dragon is, 
> that green dragon seems to be using some compiler which passes the test
>
>   if not (is_clang and self.expectedCompilerVersion(["<", "16"])):
>
> but does not actually provide the expected debug informations. @jasonmolenda 
> Do you know which exact compiler is used on green-dragon?

The lldb incremental bot uses the last good build from the 
https://green.lab.llvm.org/green/job/clang-stage1-RA/ bot as the clang compiler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132735

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Target/Target.cpp:208
+// of the debugee.
+m_scratch_type_system_map.Clear();
 m_process_sp.reset();

Michael137 wrote:
> kastiglione wrote:
> > Do we have some place in the life-cycle where we can perform this only if 
> > the target has changed? Ideally this would happen when the binary has a 
> > different timestamp, or for mach-o a different UUID.
> There is `DidUnloadModules` which gets notified when an lldb_private::Module 
> gets unloaded (e.g., on rebuilt). But this includes JITted modules (e.g., 
> when running AppleObjectiveCRuntimeV2 utility functions) in which case we 
> wouldn’t want to flush the type systems. Also the Clang REPL (and I assume 
> the Swift REPL) rely on the type system being still present after we unloaded 
> the module associated with the evaluated expression. All this is to say, I 
> found it to be quite fiddly to determine when to flush the persistent 
> variables from within that notification. Really we would like a notification 
> to the Target which says “we restarted the debugee AND some module got 
> rebuilt”. In that case it’s not safe to keep the persistent variables around. 
> I’ll double check if there isn’t something like that around.
I suppose we could do something like:

```
void Target::ModulesDidUnload(ModuleList _list, bool delete_locations) {
...
if each module in module_list is Type::eTypeExecutable or 
Type::eTypeObjectFile {
m_scratch_type_system_map.Clear();
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Target/Target.cpp:207
+// E.g., this could happen on rebuild & relaunch
+// of the debugee.
+m_scratch_type_system_map.Clear();

Michael137 wrote:
> aprantl wrote:
> > Not opposed to this, but this is leaking an implementation detail of 
> > TypeSystemClang into Target. Just out of curiosity — would if be feasible 
> > to use weak_ptr in DeclOrigin or is that defined inside of Clang and can't 
> > be changed?
> I agree with your point about leaking TypeSystem implementation details into 
> Process lifecycle. I briefly considered converting `DeclOrigin`s to hold weak 
> pointers but unfortunately the pointers are handed out by `clang::Decl`. 
> Though thinking about it, `clang::ASTContext`s are refcounted (via 
> `llvm::RefCountedBase`). So perhaps we could keep them alive that way. Though 
> I’m concerned that we then commit to supporting multiple definitions of a 
> type in the scratch AST and never really properly clearing the `DeclOrigin` 
> structures. But I’ll play around with the idea
Hmm this becomes a bit awkward because `TypeSystemClang` owns the backing 
ASTContext uniquely: `std::unique_ptr m_ast_up`.

So we can't bump the refcount unless we somehow prevent the `m_ast_up` deleter 
from kicking in when the backing `lldb_private::Module` gets destroyed.

Stepping back a little, essentially what we want is to:
1. Either, accept the fact that the `OriginMap` can contain stale pointers and 
provide a mechanism to detect this (and clear them out of the map)
2. OR, never allow a situation where we have stale pointers in this map.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D138271: Fix license header in TraceHTR.h

[Augie Fackler via Phabricator via lldb-commits]

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

Looks obviously correct to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138271

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

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG77b220524541: [lldb][DataFormatter] Add 
std::ranges::ref_view formatter (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138558

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile
  
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/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/main.cpp
@@ -0,0 +1,27 @@
+#include 
+#include 
+#include 
+#include 
+
+using string_vec = std::vector;
+
+string_vec svec{"First", "Second", "Third", "Fourth"};
+
+struct Foo {
+  string_vec vec = svec;
+};
+
+int main() {
+  {
+auto single = std::ranges::ref_view(svec[0]);
+auto all = std::views::all(svec);
+auto subset = all | std::views::take(2);
+std::puts("Break here");
+  }
+
+  {
+Foo f[2];
+auto view = std::ranges::ref_view(f);
+std::puts("Break here");
+  }
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
@@ -0,0 +1,65 @@
+"""
+Test LLDB's std::ranges::ref_view formatter
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LibcxxRangesRefViewDataFormatterTestCase(TestBase):
+
+def check_string_vec_children(self):
+return [ValueCheck(name='[0]', summary='"First"'),
+ValueCheck(name='[1]', summary='"Second"'),
+ValueCheck(name='[2]', summary='"Third"'),
+ValueCheck(name='[3]', summary='"Fourth"')]
+
+def check_string_vec_ref_view(self):
+return ValueCheck(
+name='*__range_',
+summary='size=4',
+children=self.check_string_vec_children())
+
+def check_foo(self):
+return ValueCheck(
+name='vec',
+children=self.check_string_vec_children())
+
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test that std::ranges::ref_view is formatted correctly when printed.
+"""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, 'Break here', lldb.SBFileSpec('main.cpp', False))
+
+# Check ref_view over a std::string
+self.expect_var_path('single',
+ children=[ValueCheck(
+   name='*__range_',
+   summary='"First"')])
+
+# Check all_view, which is a ref_view in this case
+self.expect_var_path('all',
+ children=[self.check_string_vec_ref_view()])
+
+# Check take_view format. Embeds a ref_view
+self.expect_var_path('subset',
+ children=[
+ ValueCheck(children=[self.check_string_vec_ref_view()]),
+ ValueCheck(name='__count_', value='2')
+ ])
+
+lldbutil.continue_to_breakpoint(self.process(), bkpt)
+
+# Check ref_view over custom type 'struct Foo'
+self.expect_var_path('view',
+ children=[ValueCheck(
+name='*__range_',
+children=[
+ValueCheck(name='[0]', type='Foo', children=[self.check_foo()]),
+ValueCheck(name='[1]', type='Foo', children=[self.check_foo()])
+])
+   ])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile
@@ -0,0 +1,6 @@
+USE_LIBCPP := 

[Lldb-commits] [lldb] 77b2205 - [lldb][DataFormatter] Add std::ranges::ref_view formatter

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-11-30T14:39:39Z
New Revision: 77b220524541a9c29a6b50caa9f65addffeb37ba

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

LOG: [lldb][DataFormatter] Add std::ranges::ref_view formatter

This patch adds a formatter for `std::ranges::ref_view`.
It simply holds a `T*`, so all this formatter does is dereference
this pointer and format it as `T` would be.

**Testing**

* Added API tests

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

Added: 
lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile

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/main.cpp

Modified: 
lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.h

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt 
b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
index 3e76ba30ada59..21108b27896a1 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
+++ b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
@@ -12,6 +12,7 @@ add_lldb_library(lldbPluginCPlusPlusLanguage PLUGIN
   LibCxxList.cpp
   LibCxxMap.cpp
   LibCxxQueue.cpp
+  LibCxxRangesRefView.cpp
   LibCxxSpan.cpp
   LibCxxTuple.cpp
   LibCxxUnorderedMap.cpp

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 752e68c73f875..97b62b4601669 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -835,6 +835,12 @@ static void LoadLibCxxFormatters(lldb::TypeCategoryImplSP 
cpp_category_sp) {
   "libc++ std::span synthetic children",
   ConstString("^std::__[[:alnum:]]+::span<.+>(( )?&)?$"), stl_deref_flags,
   true);
+  AddCXXSynthetic(
+  cpp_category_sp,
+  lldb_private::formatters::LibcxxStdRangesRefViewSyntheticFrontEndCreator,
+  "libc++ std::ranges::ref_view synthetic children",
+  ConstString("^std::__[[:alnum:]]+::ranges::ref_view<.+>(( )?&)?$"),
+  stl_deref_flags, true);
 
   cpp_category_sp->AddTypeSynthetic(
   "^(std::__[[:alnum:]]+::)deque<.+>(( )?&)?$", eFormatterMatchRegex,

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
index 96b4c10f32cea..9ab22153a92a8 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -256,6 +256,10 @@ SyntheticChildrenFrontEnd *
 LibcxxStdSpanSyntheticFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxStdRangesRefViewSyntheticFrontEndCreator(CXXSyntheticChildren *,
+   lldb::ValueObjectSP);
+
 } // namespace formatters
 } // namespace lldb_private
 

diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
new file mode 100644
index 0..6aeb557a95ff3
--- /dev/null
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
@@ -0,0 +1,87 @@
+//===-- LibCxxRangesRefView.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 "LibCxx.h"
+
+#include "lldb/Core/ValueObject.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Utility/ConstString.h"
+#include "llvm/ADT/APSInt.h"
+
+using namespace lldb;
+using namespace lldb_private;
+using namespace lldb_private::formatters;
+
+namespace lldb_private {
+namespace formatters {
+
+class LibcxxStdRangesRefViewSyntheticFrontEnd
+: public SyntheticChildrenFrontEnd {
+public:
+  LibcxxStdRangesRefViewSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
+
+  ~LibcxxStdRangesRefViewSyntheticFrontEnd() override = default;
+
+  size_t CalculateNumChildren() override {
+// __range_ will be the sole child of this type
+return 1;
+  }
+
+  lldb::ValueObjectSP GetChildAtIndex(size_t idx) override {
+// Since we only have a single child, return it
+assert(idx == 0);
+return m_range_sp;
+  }
+
+  bool 

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

[Michael Buch via Phabricator via lldb-commits]

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



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp:41
+private:
+  lldb::ValueObjectSP m_range_sp = nullptr; ///< Pointer to the dereferenced
+///< __range_ member

aprantl wrote:
> Personally I prefer 
> ```
> ///Pointer to the dereferenced __range_ member.
> lldb::ValueObjectSP m_range_sp = nullptr;
> ```
> for longer comments.
Done



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp:55
+  return 1;
+}
+

aprantl wrote:
> I wonder if it would be more readable to move those one-line function 
> definitions into the declaration?
Looks better, done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138558

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

[Lldb-commits] [PATCH] D138558: [lldb][DataFormatter] Add std::ranges::ref_view formatter

[Michael Buch via Phabricator via lldb-commits]

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

- Move short definitions inline
- Fix doxygen comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138558

Files:
  lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.h
  lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile
  
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/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/main.cpp
@@ -0,0 +1,27 @@
+#include 
+#include 
+#include 
+#include 
+
+using string_vec = std::vector;
+
+string_vec svec{"First", "Second", "Third", "Fourth"};
+
+struct Foo {
+  string_vec vec = svec;
+};
+
+int main() {
+  {
+auto single = std::ranges::ref_view(svec[0]);
+auto all = std::views::all(svec);
+auto subset = all | std::views::take(2);
+std::puts("Break here");
+  }
+
+  {
+Foo f[2];
+auto view = std::ranges::ref_view(f);
+std::puts("Break here");
+  }
+}
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/TestDataFormatterLibcxxRangesRefView.py
@@ -0,0 +1,65 @@
+"""
+Test LLDB's std::ranges::ref_view formatter
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class LibcxxRangesRefViewDataFormatterTestCase(TestBase):
+
+def check_string_vec_children(self):
+return [ValueCheck(name='[0]', summary='"First"'),
+ValueCheck(name='[1]', summary='"Second"'),
+ValueCheck(name='[2]', summary='"Third"'),
+ValueCheck(name='[3]', summary='"Fourth"')]
+
+def check_string_vec_ref_view(self):
+return ValueCheck(
+name='*__range_',
+summary='size=4',
+children=self.check_string_vec_children())
+
+def check_foo(self):
+return ValueCheck(
+name='vec',
+children=self.check_string_vec_children())
+
+@add_test_categories(["libc++"])
+def test_with_run_command(self):
+"""Test that std::ranges::ref_view is formatted correctly when printed.
+"""
+self.build()
+(self.target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, 'Break here', lldb.SBFileSpec('main.cpp', False))
+
+# Check ref_view over a std::string
+self.expect_var_path('single',
+ children=[ValueCheck(
+   name='*__range_',
+   summary='"First"')])
+
+# Check all_view, which is a ref_view in this case
+self.expect_var_path('all',
+ children=[self.check_string_vec_ref_view()])
+
+# Check take_view format. Embeds a ref_view
+self.expect_var_path('subset',
+ children=[
+ ValueCheck(children=[self.check_string_vec_ref_view()]),
+ ValueCheck(name='__count_', value='2')
+ ])
+
+lldbutil.continue_to_breakpoint(self.process(), bkpt)
+
+# Check ref_view over custom type 'struct Foo'
+self.expect_var_path('view',
+ children=[ValueCheck(
+name='*__range_',
+children=[
+ValueCheck(name='[0]', type='Foo', children=[self.check_foo()]),
+ValueCheck(name='[1]', type='Foo', children=[self.check_foo()])
+])
+   ])
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/ranges/ref_view/Makefile
@@ -0,0 +1,6 @@
+USE_LIBCPP := 1
+
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := 

[Lldb-commits] [PATCH] D138724: [lldb][Target] Flush the scratch TypeSystem when process gets deleted

[Michael Buch via Phabricator via lldb-commits]

Michael137 added inline comments.



Comment at: lldb/source/Target/Target.cpp:207
+// E.g., this could happen on rebuild & relaunch
+// of the debugee.
+m_scratch_type_system_map.Clear();

aprantl wrote:
> Not opposed to this, but this is leaking an implementation detail of 
> TypeSystemClang into Target. Just out of curiosity — would if be feasible to 
> use weak_ptr in DeclOrigin or is that defined inside of Clang and can't be 
> changed?
I agree with your point about leaking TypeSystem implementation details into 
Process lifecycle. I briefly considered converting `DeclOrigin`s to hold weak 
pointers but unfortunately the pointers are handed out by `clang::Decl`. Though 
thinking about it, `clang::ASTContext`s are refcounted (via 
`llvm::RefCountedBase`). So perhaps we could keep them alive that way. Though 
I’m concerned that we then commit to supporting multiple definitions of a type 
in the scratch AST and never really properly clearing the `DeclOrigin` 
structures. But I’ll play around with the idea



Comment at: lldb/source/Target/Target.cpp:208
+// of the debugee.
+m_scratch_type_system_map.Clear();
 m_process_sp.reset();

kastiglione wrote:
> Do we have some place in the life-cycle where we can perform this only if the 
> target has changed? Ideally this would happen when the binary has a different 
> timestamp, or for mach-o a different UUID.
There is `DidUnloadModules` which gets notified when an lldb_private::Module 
gets unloaded (e.g., on rebuilt). But this includes JITted modules (e.g., when 
running AppleObjectiveCRuntimeV2 utility functions) in which case we wouldn’t 
want to flush the type systems. Also the Clang REPL (and I assume the Swift 
REPL) rely on the type system being still present after we unloaded the module 
associated with the evaluated expression. All this is to say, I found it to be 
quite fiddly to determine when to flush the persistent variables from within 
that notification. Really we would like a notification to the Target which says 
“we restarted the debugee AND some module got rebuilt”. In that case it’s not 
safe to keep the persistent variables around. I’ll double check if there isn’t 
something like that around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138724

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

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

[Adrian Vogelsgesang via Phabricator via lldb-commits]

avogelsgesang added a comment.

The issue with this change was that if devirtualization is failing, then in the 
line

  lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
"promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);

the `promise_type` is `void`. Creating an object of type void is obviously not 
possible.

Adding a simple additional `!promise_type.isVoid()` check will fix this. 
Waiting for resolution of the test failure on https://reviews.llvm.org/D132735 
first, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132815

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

[Lldb-commits] [PATCH] D132735: [LLDB] Recognize `std::noop_coroutine()` in `std::coroutine_handle` pretty printer

[Adrian Vogelsgesang via Phabricator via lldb-commits]

avogelsgesang added a subscriber: jasonmolenda.
avogelsgesang added a comment.

As discussed via email, the reason for the test failures on green dragon is, 
that green dragon seems to be using some compiler which passes the test

  if not (is_clang and self.expectedCompilerVersion(["<", "16"])):

but does not actually provide the expected debug informations. @jasonmolenda Do 
you know which exact compiler is used on green-dragon?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132735

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

[Lldb-commits] [PATCH] D138939: [WIP][clang] adds a way to provide user-oriented reasons

[Emilia Dreamer via Phabricator via lldb-commits]

rymiel added inline comments.



Comment at: clang/include/clang/Frontend/DiagnosticRenderer.h:130
   /// \param Message The diagnostic message to emit.
+  /// \param Reason Supplementary information for the message.
   /// \param Ranges The underlined ranges for this code snippet.

Which parameter is this doxygen comment referring to?



Comment at: clang/tools/clang-format/ClangFormat.cpp:397
+  Info.FormatSummary(vec);
+  Info.FormatLegacyReason(vec);
 errs() << "clang-format error:" << vec << "\n";

I don't think this indent change was intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138939

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