[Lldb-commits] [PATCH] D152364: [lldb] Rate limit progress reports -- different approach [WIP-ish]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D152364#4406589 , @JDevlieghere 
wrote:

> Continuing some of the discussion from D150805 
>  here as it mostly relates to this patch:
>
> In D150805#4402906 , @labath wrote:
>
>> I agree that we should have a rate limiting mechanism very close to the 
>> source, to avoid wasting work for events that aren't going to be used.
>
> I'm not sure I (fully) agree with this statement. I may have misunderstood 
> the motivation for D150805 , but my 
> understanding was that it was the consumption of the progress events that was 
> too costly, not the generation. Of course this will indirectly make that 
> problem better because of the rate limiting, but as I said in the original 
> review, that still seems like something the consumer should have control over.
>
> I would have assumed that reporting the progress wouldn't be that expensive. 
> Based on the description of this patch, it seems like it's non-trivial, but 
> also not a dominating factor by any means.

That is correct. It is definitely not as slow as I expected it to be at first.

>> This is particularly important for debug info parsing, where we have 
>> multiple threads working in parallel and taking a lock even just to check 
>> whether we should report something could be a choke point.
>
> Fair enough, but this could be achieved without the rate limiting: the 
> reporting could be an async operation with the thread reporting every event.

That's a bit tricky. If you want to guarantee you don't lose any events *and* 
also not block the "sending" thread , you have to have some sort of a queuing 
mechanism -- which means you're essentially reimplementing a 
listener/broadcaster pattern. I'm sure that could be implemented more 
efficiently that what our current classes do, but I don't think that would be 
time well spent. And we'd still be generating a lot of events that noone is 
going to see.

In D152364#4406927 , @JDevlieghere 
wrote:

> While generality is part of why I favor doing the rate limiting in the 
> listener, it also means that the listener can decide the rate. For example, 
> VSCode could decide they don't need rate limiting (as is the case today) 
> while the default event handler in LLDB could make a different decision (for 
> example based on whether you're in a fast TTY).

The idea seems nice in principle, but I think the implementation would be 
somewhat messy. For "normal" events, the goal usually is to send them out as 
quickly as possible, but in this case we actually want to do the opposite -- 
force a delay before the receipt (or sending) of a specific event. And as the 
same listener will be receiving multiple kinds of events, be need this to be 
configurable on a per-event basis. If I was going down this path, I guess I'd 
do it by adding some kind of a delay/frequency argument to 
`Listener::StartListeningForEvents`  function. The listener would remember the 
requested frequency for a specific type of events as well as the last time that 
this kind of an event was received, and then the broadcaster would avoid 
enqueuing these events (and waking up the listener) if the events come faster 
than that.

Doable, just slightly complicated. We'd also need to figure out what we do with 
the existing filtering mechanism -- looking at the code, I see that we already 
have the ability to send "unique" events 
(`Broadcaster::BroadcastEventIfUnique`). This is a form of rate-limiting, so I 
think it'd make sense to merge these. However:

- this behavior is controlled on the broadcaster side
- this actually keeps the first duplicated event, whereas we'd most likely 
prefer to keep the last one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152364

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

[Lldb-commits] [PATCH] D152364: [lldb] Rate limit progress reports -- different approach [WIP-ish]

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Actually, thinking about these "unique" events, I think it would be worth 
trying out sending the progress update events as "unique". Depending on where 
exactly in the pipeline the congestion happens, it might just be enough to fix 
the immediate problem. If the slow screen updates cause the `write(stdout)` 
calls to block, then we will have a queue forming in the listener object. The 
uniqueness property would help with that as it would collapse that queue into a 
single event. If the write does not block (and the queue forms somewhere down 
the line), then we need rate limiting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152364

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

[Lldb-commits] [PATCH] D152516: [lldb][AArch64] Add thread local storage tpidr register

[David Spickett via Phabricator via lldb-commits]

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

This register is used as the pointer to the current thread
local storage block and is read from NT_ARM_TLS on Linux.

Though tpidr will be present on all AArch64 Linux, I am soon
going to add a second register tpidr2 to this set.

tpidr is only present when SME is implemented, therefore the
NT_ARM_TLS set will change size. This is why I've added this
as a dynamic register set to save changes later.

As we can't predict exactly where the TLS data will be,
the tests modify the pointer and observe changes in the program's
behaviour based on that.

So if we read the wrong value in the first place, our modifications
wouldn't work as expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152516

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c

Index: lldb/test/API/linux/aarch64/tls_register/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_register/main.c
@@ -0,0 +1,25 @@
+#include 
+#include 
+
+struct __attribute__((__packed__)) TestStruct {
+  uint64_t one;
+  uint64_t two;
+};
+
+int main() {
+  static __thread struct TestStruct test_data;
+  test_data.one = 0xABABABABCDCDCDCD;
+#define TWO_VALUE 0xEFEFEFEF01010101
+  test_data.two = TWO_VALUE;
+
+  // Barrier to ensure the above writes are done first.
+  __asm__ volatile("" ::: "memory"); // Set break point at this line.
+
+  // Here lldb moves the TLS pointer 8 bytes forward.
+  // So this actually reads test_data.two instead of test_data.one.
+  volatile bool tls_has_moved = test_data.one == TWO_VALUE;
+
+  return 0; // Set break point 2 at this line.
+  // lldb will reset the thread pointer here so we don't potentially confuse
+  // libc.
+}
Index: lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
@@ -0,0 +1,77 @@
+"""
+Test lldb's ability to read and write the AArch64 TLS register tpidr.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxTLSRegister(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_tls(self):
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point at this line."),
+num_expected_locations=1,
+)
+
+lldbutil.run_break_set_by_file_and_line(
+self,
+"main.c",
+line_number("main.c", "// Set break point 2 at this line."),
+num_expected_locations=1,
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+# We can't know what the value should be since it moves about between
+# runs. So we'll check that we can read it, and by modifying it, see
+# changes in the program.
+
+regs = self.thread().GetSelectedFrame().GetRegisters()
+tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
+self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
+tpidr = tls_regs.GetChildMemberWithName("tpidr")
+self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tpidr = tpidr.GetValueAsUnsigned()
+
+# We should be able to find a known value in the TLS area it points to.
+# test_data should be very soon after the header, 64 bytes seems to work fine.
+self.expect("memory find -e 0xABABABABCDCDCDCD $tpidr $tpidr+64",
+substrs=["data found at location: 0x"])
+
+# Now modify the TLS pointer so that anyone reading test_data.one actually
+# reads test_data.two.
+self.expect("register write tpidr 0x{:x}".format(tpidr+8))
+
+# Let the program read test_data.one.
+ 

[Lldb-commits] [PATCH] D152519: [lldb][AArch64] Add Scalable Matrix Extension option to QEMU launch script

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added subscribers: ctetreau, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added subscribers: lldb-commits, alextsao1999.
Herald added a project: LLDB.

The Scalable Matrix Extension (SME) does not require extra options
beyond setting the cpu to "max".

https://qemu-project.gitlab.io/qemu/system/arm/cpu-features.html#sme-cpu-property-examples

SME depends on SVE, so that will be enabled too even if you don't ask
for it by name.

--sve --sme -> SVE and SME
--sme   -> SVE and SME
--sve   -> Only SVE


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152519

Files:
  lldb/docs/use/qemu-testing.rst
  lldb/scripts/lldb-test-qemu/run-qemu.sh


Index: lldb/scripts/lldb-test-qemu/run-qemu.sh
===
--- lldb/scripts/lldb-test-qemu/run-qemu.sh
+++ lldb/scripts/lldb-test-qemu/run-qemu.sh
@@ -6,7 +6,8 @@
   echo -e "  --help\t\t\tDisplay this information."
   echo -e "  --arch {arm|arm64}\t\tSelects architecture QEMU system emulation."
   echo -e "  --sve\t\t\t\tEnables AArch64 SVE mode."
-  echo -e "  --mte\t\t\t\tEnables AArch64 MTE mode.\n"
+  echo -e "  --mte\t\t\t\tEnables AArch64 MTE mode."
+  echo -e "  --sme\t\t\t\tEnables AArch64 SME mode."
   echo -e "  --rootfs {path}\t\tPath of root file system image."
   echo -e "  --qemu {path}\t\t\tPath of pre-installed qemu-system-* 
executable."
   echo -e "  --kernel {path}\t\tPath of Linux kernel prebuilt image.\n"
@@ -50,6 +51,7 @@
 --qemu) QEMU_BIN=$2; shift;;
 --sve)  SVE=1;;
 --mte)  MTE=1;;
+--sme)  SME=1;;
 --help) print_usage 0 ;;
 *)  invalid_arg "$1" ;;
   esac
@@ -104,16 +106,19 @@
   if [[ $MTE ]]; then
 echo "warning: --mte is supported by AArch64 targets only"
   fi
+  if [[ $SME ]]; then
+echo "warning: --sme is supported by AArch64 targets only"
+  fi
 elif [[ "$ARCH" == "arm64" ]]; then
   QEMU_MACHINE=virt
   QEMU_SVE_MAX_VQ=4
   QEMU_CPU="cortex-a53"
 
-  if [[ $SVE ]] || [[ $MTE ]]; then
+  if [[ $SVE ]] || [[ $MTE ]] || [[ $SME ]]; then
 QEMU_CPU="max"
   fi
 
-  if [[ $SVE ]]; then
+  if [[ $SVE ]] || [[ $SME ]]; then
 QEMU_CPU="$QEMU_CPU,sve-max-vq=$QEMU_SVE_MAX_VQ"
   fi
   if [[ $MTE ]]; then
Index: lldb/docs/use/qemu-testing.rst
===
--- lldb/docs/use/qemu-testing.rst
+++ lldb/docs/use/qemu-testing.rst
@@ -93,8 +93,11 @@
 
 * --sve option will enable AArch64 SVE mode.
 
-* --mte option will enable AArch64 MTE (memory tagging) mode.
-  (can be used on its own or in addition to --sve)
+* --sme option will enable AArch64 SME mode (SME requires SVE, so this will 
also
+  be enabled).
+
+* --mte option will enable AArch64 MTE (memory tagging) mode
+  (can be used on its own or in addition to --sve).
 
 
 **Example:** Run QEMU Arm or AArch64 system emulation using run-qemu.sh


Index: lldb/scripts/lldb-test-qemu/run-qemu.sh
===
--- lldb/scripts/lldb-test-qemu/run-qemu.sh
+++ lldb/scripts/lldb-test-qemu/run-qemu.sh
@@ -6,7 +6,8 @@
   echo -e "  --help\t\t\tDisplay this information."
   echo -e "  --arch {arm|arm64}\t\tSelects architecture QEMU system emulation."
   echo -e "  --sve\t\t\t\tEnables AArch64 SVE mode."
-  echo -e "  --mte\t\t\t\tEnables AArch64 MTE mode.\n"
+  echo -e "  --mte\t\t\t\tEnables AArch64 MTE mode."
+  echo -e "  --sme\t\t\t\tEnables AArch64 SME mode."
   echo -e "  --rootfs {path}\t\tPath of root file system image."
   echo -e "  --qemu {path}\t\t\tPath of pre-installed qemu-system-* executable."
   echo -e "  --kernel {path}\t\tPath of Linux kernel prebuilt image.\n"
@@ -50,6 +51,7 @@
 --qemu) QEMU_BIN=$2; shift;;
 --sve)  SVE=1;;
 --mte)  MTE=1;;
+--sme)  SME=1;;
 --help) print_usage 0 ;;
 *)  invalid_arg "$1" ;;
   esac
@@ -104,16 +106,19 @@
   if [[ $MTE ]]; then
 echo "warning: --mte is supported by AArch64 targets only"
   fi
+  if [[ $SME ]]; then
+echo "warning: --sme is supported by AArch64 targets only"
+  fi
 elif [[ "$ARCH" == "arm64" ]]; then
   QEMU_MACHINE=virt
   QEMU_SVE_MAX_VQ=4
   QEMU_CPU="cortex-a53"
 
-  if [[ $SVE ]] || [[ $MTE ]]; then
+  if [[ $SVE ]] || [[ $MTE ]] || [[ $SME ]]; then
 QEMU_CPU="max"
   fi
 
-  if [[ $SVE ]]; then
+  if [[ $SVE ]] || [[ $SME ]]; then
 QEMU_CPU="$QEMU_CPU,sve-max-vq=$QEMU_SVE_MAX_VQ"
   fi
   if [[ $MTE ]]; then
Index: lldb/docs/use/qemu-testing.rst
===
--- lldb/docs/use/qemu-testing.rst
+++ lldb/docs/use/qemu-testing.rst
@@ -93,8 +93,11 @@
 
 * --sve option will enable AArch64 SVE mode.
 
-* --mte option will enable AArch64 MTE (memory tagging) mode.
-  (can be used on its own or in addition to --sve)
+* --sme option will enable AArch64 SME mode (SME requires SVE, s

[Lldb-commits] [PATCH] D152519: [lldb][AArch64] Add Scalable Matrix Extension option to QEMU launch script

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: omjavaid.
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

Strictly you could just use `--sve`, but it's nice to have an explicit option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152519

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

[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Sebastian Neubauer via Phabricator via lldb-commits]

sebastian-ne added inline comments.



Comment at: cmake/Modules/GetClangResourceDir.cmake:15
+  else()
+string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
+set(ret_dir lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION_MAJOR})

This fails in our downstream project.
We use add_subdirectory() to include LLVM and our parent project already sets 
PACKAGE_VERSION to something that is not the LLVM version.

Parsing PACKAGE_VERSION only if CLANG_VERSION_MAJOR is not already known seems 
to work:
```
if (NOT CLANG_VERSION_MAJOR)
  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
endif()
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[Lldb-commits] [PATCH] D152476: [lldb] Remove lldb's DWARFAbbreviationDeclarationSet in favor of llvm's

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

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

This is awesome! I believe you said there was no measurable perf diff?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152476

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

[Lldb-commits] [PATCH] D152409: [lldb] Never print children if the max depth has been reached

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

Would that be testable by implementing a python data formatter for a C struct 
that unconditionally returns an error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152409

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

[Lldb-commits] [PATCH] D152476: [lldb] Remove lldb's DWARFAbbreviationDeclarationSet in favor of llvm's

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added a comment.

Nice!




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:61
 void DWARFDebugAbbrev::GetUnsupportedForms(
 std::set &invalid_forms) const {
+  for (const auto &pair : m_abbrevCollMap) {

at some point we could modernize this into
```
llvm::DenseSet DWARFDebugAbbrev::GetUnsupportedForms()
```



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:62
 std::set &invalid_forms) const {
-  for (const auto &pair : m_abbrevCollMap)
-pair.second.GetUnsupportedForms(invalid_forms);
+  for (const auto &pair : m_abbrevCollMap) {
+for (const auto &decl : pair.second) {

nit: LLVM coding style would probably elide the {}



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h:23
 
 typedef std::map
 DWARFAbbreviationDeclarationCollMap;

for a later commit: this should probably be a DenseMap for better performance?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152476

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

[Lldb-commits] [PATCH] D152560: Make TestStepNodebug more robust against codegen changes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, Michael137.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
aprantl requested review of this revision.

Depending on the compiler I've seen that for example the step-in command can do 
a column-step first before exiting out of the function:

  * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  frame #0: 0x00013d80 
a.out`called_from_nodebug_actual(some_value=5) at with-debug.
  c:10:10
 2
 3typedef int (*debug_callee) (int);
 4
 5extern int no_debug_caller (int, debug_callee);
 6
 7int called_from_nodebug_actual(int some_value) {
 8  int return_value = 0;
 9  return_value = printf("Length: %d.\n", some_value);
  -> 10 return return_value; // Stop here and step out of me
   ^
 11   }
 12   
 13   int called_from_nodebug(int some_value) {
 14 int intermediate_return_value = 0;
 15 intermediate_return_value = called_from_nodebug_actual(some_value);
 16 return intermediate_return_value;
 17   }
 18
  Process 58672 launched: 
'/Volumes/Data/llvm-project/_build.ninja.release/lldb-test-build.noindex/functionalities/step-avoids-no-debug/TestStepNoDebug.test_step_out_with_python_dsym/a.out'
 (arm64)
  (lldb) s
  Process 58672 stopped
  * thread #1, queue = 'com.apple.main-thread', stop reason = step in
  frame #0: 0x00013d8c 
a.out`called_from_nodebug_actual(some_value=5) at with-debug.c:10:3
 2
 3typedef int (*debug_callee) (int);
 4
 5extern int no_debug_caller (int, debug_callee);
 6
 7int called_from_nodebug_actual(int some_value) {
 8  int return_value = 0;
 9  return_value = printf("Length: %d.\n", some_value);
  -> 10 return return_value; // Stop here and step out of me
^
 11   }
 12   


https://reviews.llvm.org/D152560

Files:
  lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
  lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c
  lldb/test/API/functionalities/step-avoids-no-debug/without-debug.c

Index: lldb/test/API/functionalities/step-avoids-no-debug/without-debug.c
===
--- lldb/test/API/functionalities/step-avoids-no-debug/without-debug.c
+++ lldb/test/API/functionalities/step-avoids-no-debug/without-debug.c
@@ -1,16 +1,12 @@
-typedef int (*debug_callee) (int);
+typedef int (*debug_callee)(int);
 
-int
-no_debug_caller_intermediate(int input, debug_callee callee)
-{
+int no_debug_caller_intermediate(int input, debug_callee callee) {
   int return_value = 0;
   return_value = callee(input);
   return return_value;
 }
 
-int
-no_debug_caller (int input, debug_callee callee)
-{
+int no_debug_caller(int input, debug_callee callee) {
   int return_value = 0;
   return_value = no_debug_caller_intermediate (input, callee);
   return return_value;
Index: lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c
===
--- lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c
+++ lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c
@@ -4,26 +4,20 @@
 
 extern int no_debug_caller (int, debug_callee);
 
-int
-called_from_nodebug_actual(int some_value)
-{
+int called_from_nodebug_actual(int some_value) {
   int return_value = 0;
-  return_value  = printf ("Length: %d.\n", some_value);
+  return_value = printf("Length: %d.\n", some_value);
   return return_value; // Stop here and step out of me
 }
 
-int
-called_from_nodebug(int some_value)
-{
+int called_from_nodebug(int some_value) {
   int intermediate_return_value = 0;
   intermediate_return_value = called_from_nodebug_actual(some_value);
   return intermediate_return_value;
 }
 
-int
-main()
-{
+int main() {
   int return_value = no_debug_caller(5, called_from_nodebug);
-  printf ("I got: %d.\n", return_value);
+  printf("I got: %d.\n", return_value);
   return 0;
 }
Index: lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
===
--- lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
+++ lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
@@ -106,9 +106,9 @@
 self.thread = threads[0]
 
 def do_step_out_past_nodebug(self):
-# The first step out takes us to the called_from_nodebug frame, just to make sure setting
-# step-out-avoid-nodebug doesn't change the behavior in frames with
-# debug info.
+# The first step out takes us to the called_from_nodebug
+# frame, just to make sure setting step-out-avoid-nodebug
+# doesn't change the behavior in frames with debug info.
 self.thread.StepOut()
 self.hit_correct_line(
 "intermediate_r

[Lldb-commits] [PATCH] D152476: [lldb] Remove lldb's DWARFAbbreviationDeclarationSet in favor of llvm's

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D152476#4408654 , @fdeazeve wrote:

> This is awesome! I believe you said there was no measurable perf diff?

Yeah, my initial experiments measured no significant perf difference no matter 
how you built lldb. llvm's and lldb's `DWARFAbbreviationDeclarationSet` 
implementations both use llvm's `DWARFAbbreviationDeclaration` and have the 
same semantics around their extraction. I intentionally made both 
implementations behave the exact same so that this change would be as simple as 
possible.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp:62
 std::set &invalid_forms) const {
-  for (const auto &pair : m_abbrevCollMap)
-pair.second.GetUnsupportedForms(invalid_forms);
+  for (const auto &pair : m_abbrevCollMap) {
+for (const auto &decl : pair.second) {

aprantl wrote:
> nit: LLVM coding style would probably elide the {}
Will update, thanks!



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h:23
 
 typedef std::map
 DWARFAbbreviationDeclarationCollMap;

aprantl wrote:
> for a later commit: this should probably be a DenseMap for better performance?
Possibly? It would be interesting to switch away from std::map to a different 
type (like DenseMap) to measure performance. Either way, I also want to switch 
us to use `llvm::DWARFDebugAbbrev`, so it might be worth doing that work first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152476

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

[Lldb-commits] [PATCH] D152476: [lldb] Remove lldb's DWARFAbbreviationDeclarationSet in favor of llvm's

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 530021.
bulbazord added a comment.

Adjust formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152476

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -16,7 +16,6 @@
 
 #include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDataExtractor.h"
-#include "Plugins/SymbolFile/DWARF/DWARFDebugAbbrev.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugArangeSet.h"
 #include "Plugins/SymbolFile/DWARF/DWARFDebugAranges.h"
 #include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
@@ -68,252 +67,6 @@
   EXPECT_EQ(expected_abilities, symfile->CalculateAbilities());
 }
 
-TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start1) {
-  // Test that if we have a .debug_abbrev that contains ordered abbreviation
-  // codes that start at 1, that we get O(1) access.
-
-  const auto byte_order = eByteOrderLittle;
-  const uint8_t addr_size = 4;
-  StreamString encoder(Stream::eBinary, addr_size, byte_order);
-  encoder.PutULEB128(1); // Abbrev code 1
-  encoder.PutULEB128(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_yes);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(2); // Abbrev code 2
-  encoder.PutULEB128(DW_TAG_subprogram);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(0); // Abbrev code 0 (termination)
-
-  DWARFDataExtractor data;
-  data.SetData(encoder.GetData(), encoder.GetSize(), byte_order);
-  DWARFAbbreviationDeclarationSet abbrev_set;
-  lldb::offset_t data_offset = 0;
-  llvm::Error error = abbrev_set.extract(data, &data_offset);
-  EXPECT_FALSE(bool(error));
-  // Make sure we have O(1) access to each abbreviation by making sure the
-  // index offset is 1 and not UINT32_MAX
-  EXPECT_EQ(abbrev_set.GetIndexOffset(), 1u);
-
-  auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(1);
-  EXPECT_EQ(abbrev1->getTag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->hasChildren());
-  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
-  auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(2);
-  EXPECT_EQ(abbrev2->getTag(), DW_TAG_subprogram);
-  EXPECT_FALSE(abbrev2->hasChildren());
-  EXPECT_EQ(abbrev2->getNumAttributes(), 1u);
-}
-
-TEST_F(SymbolFileDWARFTests, TestAbbrevOrder1Start5) {
-  // Test that if we have a .debug_abbrev that contains ordered abbreviation
-  // codes that start at 5, that we get O(1) access.
-
-  const auto byte_order = eByteOrderLittle;
-  const uint8_t addr_size = 4;
-  StreamString encoder(Stream::eBinary, addr_size, byte_order);
-  encoder.PutULEB128(5); // Abbrev code 5
-  encoder.PutULEB128(DW_TAG_compile_unit);
-  encoder.PutHex8(DW_CHILDREN_yes);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(6); // Abbrev code 6
-  encoder.PutULEB128(DW_TAG_subprogram);
-  encoder.PutHex8(DW_CHILDREN_no);
-  encoder.PutULEB128(DW_AT_name);
-  encoder.PutULEB128(DW_FORM_strp);
-  encoder.PutULEB128(0);
-  encoder.PutULEB128(0);
-
-  encoder.PutULEB128(0); // Abbrev code 0 (termination)
-
-  DWARFDataExtractor data;
-  data.SetData(encoder.GetData(), encoder.GetSize(), byte_order);
-  DWARFAbbreviationDeclarationSet abbrev_set;
-  lldb::offset_t data_offset = 0;
-  llvm::Error error = abbrev_set.extract(data, &data_offset);
-  EXPECT_FALSE(bool(error));
-  // Make sure we have O(1) access to each abbreviation by making sure the
-  // index offset is 5 and not UINT32_MAX
-  EXPECT_EQ(abbrev_set.GetIndexOffset(), 5u);
-
-  auto abbrev1 = abbrev_set.GetAbbreviationDeclaration(5);
-  EXPECT_EQ(abbrev1->getTag(), DW_TAG_compile_unit);
-  EXPECT_TRUE(abbrev1->hasChildren());
-  EXPECT_EQ(abbrev1->getNumAttributes(), 1u);
-  auto abbrev2 = abbrev_set.GetAbbreviationDeclaration(6);
-  EXPECT_EQ(abbrev2->getTag(), DW_TAG_subprogram);
-  EXPECT_FALSE(abbrev2->hasChildren());
-  EXPECT_EQ(abbrev2->getNumAttributes(), 1u);
-}
-
-TEST_F(SymbolFileDWARFTests, TestAbbrevOutOfOrder) {
-  // Test that if we have a .debug_abbrev that contains unordered abbreviation
-  // codes, that we can access the information correctly.
-
-  const auto byte_order = eByteOrderLit

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Alex Langford via Phabricator via lldb-commits]

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

The purpose of this tool is to be able to generate a project with an
arbitrary number of source files in a given programming language. I
believe this can be useful as it lets you benchmark a given LLDB change
for performance locally. For example, if you are changing the way LLDB
processes a specific debug info section, you may want to make sure that
you're not regressing performance (or if you're making the change for
performance, you want to ensure that it actually improves things). You
could use this tool to generate 10,000 (or more) source files and see
how quickly lldb processes the debug info of the resulting binary.

I understand that many folks will measure performance changes against
large projects, usually proprietary in nature. I believe this is quite a
valid way to measure performance changes and this tool isn't meant to
change that. I wanted to offer this tool primarily as an alternative
that gives the developer more control over the size and contents of the
project (which is not something you always have when working with
proprietary projects).

Currently this tool only supports C, C++, and Swift. The Swift portions will
only work on macOS right now and will not work with upstream lldb. There are
also multiple ways to build Swift projects, this just chooses one specific
approach. Future work could include adding an option to choose the structure of
the generated Swift project.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152569

Files:
  lldb/scripts/generate-project.py

Index: lldb/scripts/generate-project.py
===
--- /dev/null
+++ lldb/scripts/generate-project.py
@@ -0,0 +1,273 @@
+#!/usr/bin/env python3
+
+# Project generation script
+# =
+# The purpose of this script is to generate a project with an arbitrary amount
+# of source files. This is useful for testing the performance impact of a given
+# LLDB change. For example, you can use this tool to generate a project with
+# 10,000 C++ source files and see how quickly LLDB processes the resulting
+# binary and related metadata (e.g. debug info).
+
+import os
+import sys
+import typing
+
+def print_usage() -> None:
+print("Usage: generate-project.py   ")
+
+def generate_c_header(directory: str, index: int) -> None:
+header_path = f"{directory}/obj{index}.h"
+with open(header_path, "w") as f:
+f.write(f"#ifndef _OBJ{index}_H\n")
+f.write(f"#define _OBJ{index}_H\n")
+f.write(f"extern int obj{index};\n")
+f.write(f"void call_obj{index}(void);\n")
+f.write(f"#endif // _OBJ{index}_H\n")
+
+def generate_c_impl(directory: str, index: int) -> None:
+impl_path = f"{directory}/obj{index}.c"
+with open(impl_path, "w") as f:
+f.write("#include \n")
+f.write(f"#include \"obj{index}.h\"\n\n")
+f.write(f"int obj{index} = {index};\n")
+f.write(f"void call_obj{index}(void)")
+f.write(" {\n")
+f.write(f"  printf(\"%d\\n\", obj{index});\n")
+f.write("}\n")
+
+def generate_cpp_header(directory: str, index: int) -> None:
+header_path = f"{directory}/obj{index}.h"
+with open(header_path, "w") as f:
+f.write(f"#ifndef _OBJ{index}_H\n")
+f.write(f"#define _OBJ{index}_H\n")
+f.write("namespace obj {\n")
+f.write(f"class Obj{index}")
+f.write(" {\n")
+f.write("public:\n")
+f.write("  static void Call(void);\n")
+f.write("}; // class\n")
+f.write("} // namespace\n")
+f.write(f"#endif // _OBJ{index}_H\n")
+
+def generate_cpp_impl(directory: str, index: int) -> None:
+impl_path = f"{directory}/obj{index}.cpp"
+with open(impl_path, "w") as f:
+f.write("#include \n")
+f.write(f"#include \"obj{index}.h\"\n\n")
+f.write("namespace obj {\n")
+f.write(f"void Obj{index}::Call(void)")
+f.write(" {\n")
+f.write(f"  std::cout << {index} << std::endl;;\n")
+f.write("}\n")
+f.write("} // namespace\n")
+
+def generate_swift_impl(directory: str, index: int) -> None:
+impl_path = f"{directory}/obj{index}.swift"
+with open(impl_path, "w") as f:
+f.write(f"public func call_obj{index}()")
+f.write(" {\n")
+f.write(f"  print(\"{index}\")\n")
+f.write("}\n")
+
+def generate_c_driver(directory: str, number_of_objects: int) -> None:
+main_path = f"{directory}/main.c"
+with open(main_path, "w") as f:
+for i in range(0, number_of_objects):
+f.write(f"#include \"obj{i}.h\"\n")
+
+f.write("int main() {\n")
+for i in range(0, number_of_objects):
+f.wri

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added subscribers: beanz, clang-vendors.
aaron.ballman added a comment.

In D151683#4384017 , @erichkeane 
wrote:

> In D151683#4382408 , @philnik wrote:
>
>> No. I guess, since you are asking I should write one for this? Only for the 
>> removal of `-fdouble-square-bracket-attributes`, or also for back porting 
>> this?
>
> The RFC I would expect for "allow C/C++ style attributes as an extension in 
> previous modes".  This is a pretty significant feature to allow as an 
> extension, particularly since our two main alternative compilers (G++/MSVC) 
> don't have this as an extension. I'd be curious how the other C compilers do 
> this as well.

I think this does warrant an RFC because it impacts both C++ and C, but I'm 
hoping the RFC is uncontroversial. It's mostly to establish what the use cases 
are for needing the extension. The feature is conforming as an extension to 
both languages, and I don't know of any breakage that would come from enabling 
it by default. I'm not certain whether we want to remove the feature flag 
immediately or whether we'd like to give one release of warning about it being 
removed (I'll defer to whatever @MaskRay thinks is reasonable) -- that search 
is compelling for why it's safe to remove the option, but there's plenty of 
proprietary code which we don't know about, so it's possible the option is 
still used. I'd be especially curious to know if anyone is using 
`-fno-double-square-bracket-attributes` to disable the feature in a mode where 
it would otherwise be enabled. I'm adding the `clang-vendors` group as a 
subscriber as an early warning that removing the command line option could be a 
potentially breaking change.

In terms of implementation work, there's still some minor stuff to address. 
Also, please be sure to also add a release note about the changes, and a 
potentially breaking entry for removing the command line option (assuming we 
elect to go that route).




Comment at: clang/docs/LanguageExtensions.rst:1434
 Designated initializers (N494)  C99
   C89
 Array & element qualification (N2607)   C2x
   C89
+==  
= =

You need to add an entry here as well, as this also extends C.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:553
 def err_while_loop_outside_of_a_function : Error<
-  "while loop outside of a function">; 
+  "while loop outside of a function">;
 def err_brackets_go_after_unqualified_id : Error<

Spurious whitespace change.



Comment at: clang/include/clang/Driver/Options.td:3530
   Values<"none,cf,cf-nochecks">;
-def mcpu_EQ : Joined<["-"], "mcpu=">, Group, 
+def mcpu_EQ : Joined<["-"], "mcpu=">, Group,
   HelpText<"For a list of available CPUs for the target use '-mcpu=help'">;

Spurious whitespace change.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:1196
   // Add include/gpu-none-libc/* to our system include path. This lets us 
use
-  // GPU-specific system headers first. 
+  // GPU-specific system headers first.
   // TODO: We need to find a way to make these headers compatible with the

Spurious whitespace change.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4477
   SourceLocation OpenLoc = Tok.getLocation();
-  Diag(OpenLoc, diag::warn_cxx98_compat_attribute);
+  Diag(OpenLoc, getLangOpts().CPlusPlus11 ? diag::warn_cxx98_compat_attribute
+: getLangOpts().CPlusPlus ? diag::warn_ext_cxx11_attributes

Missing the same "not compatible with standards before C2x" warning as for C++ 
(might want to reword the C++ warning at the same time to fit the newer style)



Comment at: clang/test/OpenMP/assumes_messages_attr.c:57
 [[omp::directive(begin assumes ext_abc)]];
-

Spurious removal.



Comment at: clang/test/Parser/asm.c:14
   asm("foo" : "=r" (a)); // expected-error {{use of undeclared identifier 'a'}}
-  asm("foo" : : "r" (b)); // expected-error {{use of undeclared identifier 
'b'}} 
+  asm("foo" : : "r" (b)); // expected-error {{use of undeclared identifier 
'b'}}
 

Spurious whitespace change



Comment at: clang/test/Parser/cxx-decl.cpp:316
 #if __cplusplus >= 201103L
-// expected-error@+3 {{expected}}
+// expected-error@+2 {{expected}}
 // expected-error@-3 {{expected ';' after top level declarator}}

Huh... I wasn't expecting to see a change here because there's no attribute 
nearby. Probably fine, but still a bit curious.



Comment at: clang/test/Parser/cxx-decl.cpp:322
 #endif
-

Spurious change.



Comm

[Lldb-commits] [PATCH] D152573: [lldb][NFCI] Remove use of ConstString from Listener

[Alex Langford via Phabricator via lldb-commits]

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

The only place ConstString was used in Listener was for filtering
broadcasters by name when looking for the next event. This functionality
is completely unused from what I can tell (even in downstream forks).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152573

Files:
  lldb/include/lldb/Utility/Listener.h
  lldb/source/Utility/Listener.cpp

Index: lldb/source/Utility/Listener.cpp
===
--- lldb/source/Utility/Listener.cpp
+++ lldb/source/Utility/Listener.cpp
@@ -8,7 +8,6 @@
 
 #include "lldb/Utility/Listener.h"
 #include "lldb/Utility/Broadcaster.h"
-#include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/LLDBLog.h"
 
@@ -222,46 +221,25 @@
 
 class EventMatcher {
 public:
-  EventMatcher(Broadcaster *broadcaster, const ConstString *broadcaster_names,
-   uint32_t num_broadcaster_names, uint32_t event_type_mask)
-  : m_broadcaster(broadcaster), m_broadcaster_names(broadcaster_names),
-m_num_broadcaster_names(num_broadcaster_names),
-m_event_type_mask(event_type_mask) {}
+  EventMatcher(Broadcaster *broadcaster, uint32_t event_type_mask)
+  : m_broadcaster(broadcaster), m_event_type_mask(event_type_mask) {}
 
   bool operator()(const EventSP &event_sp) const {
 if (m_broadcaster && !event_sp->BroadcasterIs(m_broadcaster))
   return false;
 
-if (m_broadcaster_names) {
-  bool found_source = false;
-  const llvm::StringRef event_broadcaster_name =
-  event_sp->GetBroadcaster()->GetBroadcasterName();
-  for (uint32_t i = 0; i < m_num_broadcaster_names; ++i) {
-if (m_broadcaster_names[i] == event_broadcaster_name) {
-  found_source = true;
-  break;
-}
-  }
-  if (!found_source)
-return false;
-}
-
 return m_event_type_mask == 0 || m_event_type_mask & event_sp->GetType();
   }
 
 private:
   Broadcaster *m_broadcaster;
-  const ConstString *m_broadcaster_names;
-  const uint32_t m_num_broadcaster_names;
   const uint32_t m_event_type_mask;
 };
 
 bool Listener::FindNextEventInternal(
 std::unique_lock &lock,
-Broadcaster *broadcaster, // nullptr for any broadcaster
-const ConstString *broadcaster_names, // nullptr for any event
-uint32_t num_broadcaster_names, uint32_t event_type_mask, EventSP &event_sp,
-bool remove) {
+Broadcaster *broadcaster, // nullptr for any broadcaster
+uint32_t event_type_mask, EventSP &event_sp, bool remove) {
   // NOTE: callers of this function must lock m_events_mutex using a
   // Mutex::Locker
   // and pass the locker as the first argument. m_events_mutex is no longer
@@ -273,13 +251,11 @@
 
   Listener::event_collection::iterator pos = m_events.end();
 
-  if (broadcaster == nullptr && broadcaster_names == nullptr &&
-  event_type_mask == 0) {
+  if (broadcaster == nullptr && event_type_mask == 0) {
 pos = m_events.begin();
   } else {
 pos = std::find_if(m_events.begin(), m_events.end(),
-   EventMatcher(broadcaster, broadcaster_names,
-num_broadcaster_names, event_type_mask));
+   EventMatcher(broadcaster, event_type_mask));
   }
 
   if (pos != m_events.end()) {
@@ -288,12 +264,10 @@
 if (log != nullptr)
   LLDB_LOGF(log,
 "%p '%s' Listener::FindNextEventInternal(broadcaster=%p, "
-"broadcaster_names=%p[%u], event_type_mask=0x%8.8x, "
+"event_type_mask=0x%8.8x, "
 "remove=%i) event %p",
 static_cast(this), GetName(),
-static_cast(broadcaster),
-static_cast(broadcaster_names),
-num_broadcaster_names, event_type_mask, remove,
+static_cast(broadcaster), event_type_mask, remove,
 static_cast(event_sp.get()));
 
 if (remove) {
@@ -315,7 +289,7 @@
 Event *Listener::PeekAtNextEvent() {
   std::unique_lock guard(m_events_mutex);
   EventSP event_sp;
-  if (FindNextEventInternal(guard, nullptr, nullptr, 0, 0, event_sp, false))
+  if (FindNextEventInternal(guard, nullptr, 0, event_sp, false))
 return event_sp.get();
   return nullptr;
 }
@@ -323,7 +297,7 @@
 Event *Listener::PeekAtNextEventForBroadcaster(Broadcaster *broadcaster) {
   std::unique_lock guard(m_events_mutex);
   EventSP event_sp;
-  if (FindNextEventInternal(guard, broadcaster, nullptr, 0, 0, event_sp, false))
+  if (FindNextEventInternal(guard, broadcaster, 0, event_sp, false))
 return event_sp.get();
   return nullptr;
 }
@@ -333,26 +307,23 @@
 uint32_t event_type_mask) {
   std::unique_lock guar

[Lldb-commits] [lldb] 347a7b2 - [lldb][NFCI] Delete unused member from SymbolFileSymtab

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-09T11:53:52-07:00
New Revision: 347a7b2a89ce03d51a39d2380a520a27c2954af8

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

LOG: [lldb][NFCI] Delete unused member from SymbolFileSymtab

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h 
b/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
index 4cf4b5f370792..1bbc4de9c9425 100644
--- a/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
+++ b/lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h
@@ -93,14 +93,11 @@ class SymbolFileSymtab : public 
lldb_private::SymbolFileCommon {
 
   lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t index) override;
 
-  typedef std::map TypeMap;
-
   lldb_private::Symtab::IndexCollection m_source_indexes;
   lldb_private::Symtab::IndexCollection m_func_indexes;
   lldb_private::Symtab::IndexCollection m_code_indexes;
   lldb_private::Symtab::IndexCollection m_data_indexes;
   lldb_private::Symtab::NameToIndexMap m_objc_class_name_to_index;
-  TypeMap m_objc_class_types;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_SYMTAB_SYMBOLFILESYMTAB_H



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

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Chris Bieneman via Phabricator via lldb-commits]

beanz added inline comments.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

aaron.ballman wrote:
> philnik wrote:
> > Should this also get an extension warning/should attributes be disabled for 
> > HLSL?
> CC @beanz 
> 
> I was wondering the same thing. :-)
By bug rather than design DXC allows C++ attribute syntax in some places for 
HLSL.

I'm totally fine with (and prefer) following the rest of the languages here and 
having HLSL in Clang always allow C++ attributes regardless of language version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Nikolas Klauser via Phabricator via lldb-commits]

philnik added inline comments.



Comment at: clang/test/Parser/cxx-decl.cpp:316
 #if __cplusplus >= 201103L
-// expected-error@+3 {{expected}}
+// expected-error@+2 {{expected}}
 // expected-error@-3 {{expected ';' after top level declarator}}

aaron.ballman wrote:
> Huh... I wasn't expecting to see a change here because there's no attribute 
> nearby. Probably fine, but still a bit curious.
This is probably because of the whitespace trim below.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

beanz wrote:
> aaron.ballman wrote:
> > philnik wrote:
> > > Should this also get an extension warning/should attributes be disabled 
> > > for HLSL?
> > CC @beanz 
> > 
> > I was wondering the same thing. :-)
> By bug rather than design DXC allows C++ attribute syntax in some places for 
> HLSL.
> 
> I'm totally fine with (and prefer) following the rest of the languages here 
> and having HLSL in Clang always allow C++ attributes regardless of language 
> version.
Would you like some sort of warning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Chris Bieneman via Phabricator via lldb-commits]

beanz added inline comments.



Comment at: clang/test/ParserHLSL/group_shared.hlsl:14
-// expected-error@+1 {{expected expression}}
 float groupshared [[]] i = 12;
 

philnik wrote:
> beanz wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > Should this also get an extension warning/should attributes be disabled 
> > > > for HLSL?
> > > CC @beanz 
> > > 
> > > I was wondering the same thing. :-)
> > By bug rather than design DXC allows C++ attribute syntax in some places 
> > for HLSL.
> > 
> > I'm totally fine with (and prefer) following the rest of the languages here 
> > and having HLSL in Clang always allow C++ attributes regardless of language 
> > version.
> Would you like some sort of warning?
Since DXC accepts the syntax without a warning/error I think we're fine without 
one here. There won't be any portability issues with code that use C++ 
attributes in HLSL unless they go back to the _extremely_ old compiler that we 
don't really support anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

That looks useful!




Comment at: lldb/scripts/generate-project.py:175
+# Building Objects and their respective swiftmodule
+f.write(f"$(OBJDIR)/%.o: %.swift objdir\n")
+f.write("\t$(SWIFT_FE) -c -primary-file $< -I objs/ \\\n")

Would `"""` multiline literals be more readable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

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

[Lldb-commits] [PATCH] D152579: [lldb] Symtab::SectionFileAddressesChanged should clear the file address map instead of name map

[Alex Langford via Phabricator via lldb-commits]

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

Currently, `SectionFileAddressesChanged` clears out the `name_to_index`
map and sets `m_file_addr_to_index_compute` to false. This is strange,
as these two fields are used for different purposes. What we should be
doing is clearing the file address to index mapping.

There are 2 bugs here:

1. If we call SectionFileAddressesChanged after the name indexes have been 
computed, we end up with an empty name to index map, so lookups will fail. This 
doesn't happen today because we don't initialize the name indexes before 
calling this, but this is a refactor away from failing in this way.
2. Because we don't clear `m_file_addr_to_index` but still set it's computed 
flag to false, it ends up with twice the amount of information. One entry will 
be correct (since it was recalculated), one entry will be outdated.

rdar://110192434


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152579

Files:
  lldb/source/Symbol/Symtab.cpp


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -80,8 +80,7 @@
 }
 
 void Symtab::SectionFileAddressesChanged() {
-  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-  name_to_index.Clear();
+  m_file_addr_to_index.Clear();
   m_file_addr_to_index_computed = false;
 }
 


Index: lldb/source/Symbol/Symtab.cpp
===
--- lldb/source/Symbol/Symtab.cpp
+++ lldb/source/Symbol/Symtab.cpp
@@ -80,8 +80,7 @@
 }
 
 void Symtab::SectionFileAddressesChanged() {
-  auto &name_to_index = GetNameToSymbolIndexMap(lldb::eFunctionNameTypeNone);
-  name_to_index.Clear();
+  m_file_addr_to_index.Clear();
   m_file_addr_to_index_computed = false;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/scripts/generate-project.py:118-126
+f.write(f"  $(OBJDIR)/main.o\n")
+f.write("\n")
+f.write("all: many-objects\n")
+f.write("objdir:\n")
+f.write("\tmkdir -p $(OBJDIR)\n")
+f.write(f"$(OBJDIR)/%.o: %.c objdir\n")
+f.write(f"\t$(CC) -g -c -o $@ $<\n")

minor, but I noticed some of these f-strings don't have an `{}` interpolation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

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

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/scripts/generate-project.py:175
+# Building Objects and their respective swiftmodule
+f.write(f"$(OBJDIR)/%.o: %.swift objdir\n")
+f.write("\t$(SWIFT_FE) -c -primary-file $< -I objs/ \\\n")

aprantl wrote:
> Would `"""` multiline literals be more readable?
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

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

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.



Comment at: lldb/scripts/generate-project.py:69
+f.write(" {\n")
+f.write(f"  print(\"{index}\")\n")
+f.write("}\n")





Comment at: lldb/scripts/generate-project.py:116
+f.write("objects = \\\n")
+for i in range(0, number_of_objects):
+f.write(f"  $(OBJDIR)/obj{i}.o \\\n")




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

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

[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D141907#4094748 , @MaskRay wrote:

> [...]
> edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable 
> `CLANG_RESOURCE_DIR` but did not explain why. 
> In the long term, the CMake variable `CLANG_RESOURCE_DIR` probably should be 
> removed.

My feeling stays the same. In the long term, we should try removing the CMake 
variable `CLANG_RESOURCE_DIR`.
Customizing the variable in a special way will make some `clang/test/Driver` 
tests fail, I don't know it's healthy for contributors to be aware of this 
yet-another configure variable for `clang/test/Driver` tests. The 
`CLANG_RESOURCE_DIR` users should be served by specifying `-resource-dir=` in a 
configuration file 
(https://clang.llvm.org/docs/UsersManual.html#configuration-files)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[Lldb-commits] [PATCH] D152582: [lldb] Change return type of UnixSignals::GetShortName

[Alex Langford via Phabricator via lldb-commits]

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

The short names of each signal name and alias only exist as ConstStrings
in this one scenario. For example, GetShortName("SIGHUP") will just give
you "HUP". There's not a good reason the string "HUP" needs to be in the
ConstString StringPool, and that's true for just about every signal
name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152582

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


Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -195,10 +195,8 @@
   return m_signals.find(signo) != m_signals.end();
 }
 
-ConstString UnixSignals::GetShortName(ConstString name) const {
-  if (name)
-return ConstString(name.GetStringRef().substr(3)); // Remove "SIG" from 
name
-  return name;
+llvm::StringRef UnixSignals::GetShortName(llvm::StringRef name) const {
+  return name.substr(3); // Remove "SIG" from name
 }
 
 int32_t UnixSignals::GetSignalNumberFromName(const char *name) const {
Index: lldb/include/lldb/Target/UnixSignals.h
===
--- lldb/include/lldb/Target/UnixSignals.h
+++ lldb/include/lldb/Target/UnixSignals.h
@@ -77,8 +77,6 @@
 
   int32_t GetSignalAtIndex(int32_t index) const;
 
-  ConstString GetShortName(ConstString name) const;
-
   // We assume that the elements of this object are constant once it is
   // constructed, since a process should never need to add or remove symbols as
   // it runs.  So don't call these functions anywhere but the constructor of
@@ -147,6 +145,8 @@
 void Reset(bool reset_stop, bool reset_notify, bool reset_suppress);
   };
 
+  llvm::StringRef GetShortName(llvm::StringRef name) const;
+
   virtual void Reset();
 
   typedef std::map collection;


Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -195,10 +195,8 @@
   return m_signals.find(signo) != m_signals.end();
 }
 
-ConstString UnixSignals::GetShortName(ConstString name) const {
-  if (name)
-return ConstString(name.GetStringRef().substr(3)); // Remove "SIG" from name
-  return name;
+llvm::StringRef UnixSignals::GetShortName(llvm::StringRef name) const {
+  return name.substr(3); // Remove "SIG" from name
 }
 
 int32_t UnixSignals::GetSignalNumberFromName(const char *name) const {
Index: lldb/include/lldb/Target/UnixSignals.h
===
--- lldb/include/lldb/Target/UnixSignals.h
+++ lldb/include/lldb/Target/UnixSignals.h
@@ -77,8 +77,6 @@
 
   int32_t GetSignalAtIndex(int32_t index) const;
 
-  ConstString GetShortName(ConstString name) const;
-
   // We assume that the elements of this object are constant once it is
   // constructed, since a process should never need to add or remove symbols as
   // it runs.  So don't call these functions anywhere but the constructor of
@@ -147,6 +145,8 @@
 void Reset(bool reset_stop, bool reset_notify, bool reset_suppress);
   };
 
+  llvm::StringRef GetShortName(llvm::StringRef name) const;
+
   virtual void Reset();
 
   typedef std::map collection;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D152582: [lldb] Change return type of UnixSignals::GetShortName

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Don't you need to update the callers as well ? Otherwise LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152582

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

[Lldb-commits] [PATCH] D152560: Make TestStepNodebug more robust against codegen changes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

  lldb) image dump line-table with-debug.c
  Line table for 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c
 in `a.out
  0x00013d4c: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:7
  0x00013d5c: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:8:7
  0x00013d60: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:9:42
  0x00013d68: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:9:18
  0x00013d7c: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:9:16
  0x00013d80: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:10:10
  0x00013d84: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:10:3
  0x00013d90: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:13
  0x00013da0: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:14:7
  0x00013da4: 
/Volumes/Data/llvm-project/lldb/test/API/functionalities/step-avoids-no-debug/with-debug.c:15:58


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

https://reviews.llvm.org/D152560

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

[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Tom Stellard via Phabricator via lldb-commits]

tstellar added a comment.

In D141907#4409973 , @MaskRay wrote:

> In D141907#4094748 , @MaskRay wrote:
>
>> [...]
>> edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable 
>> `CLANG_RESOURCE_DIR` but did not explain why. 
>> In the long term, the CMake variable `CLANG_RESOURCE_DIR` probably should be 
>> removed.
>
> My feeling stays the same. In the long term, we should try removing the CMake 
> variable `CLANG_RESOURCE_DIR`.
> Customizing the variable in a special way will make some `clang/test/Driver` 
> tests fail, I don't know it's healthy for contributors to be aware of this 
> yet-another configure variable for `clang/test/Driver` tests. The 
> `CLANG_RESOURCE_DIR` users should be served by specifying `-resource-dir=` in 
> a configuration file 
> (https://clang.llvm.org/docs/UsersManual.html#configuration-files)

I agree with this

In D141907#4409973 , @MaskRay wrote:

> In D141907#4094748 , @MaskRay wrote:
>
>> [...]
>> edeaf16f2c2f02d6e43312d48d26d354d87913f3 (2011) added the CMake variable 
>> `CLANG_RESOURCE_DIR` but did not explain why. 
>> In the long term, the CMake variable `CLANG_RESOURCE_DIR` probably should be 
>> removed.
>
> My feeling stays the same. In the long term, we should try removing the CMake 
> variable `CLANG_RESOURCE_DIR`.
> Customizing the variable in a special way will make some `clang/test/Driver` 
> tests fail, I don't know it's healthy for contributors to be aware of this 
> yet-another configure variable for `clang/test/Driver` tests. The 
> `CLANG_RESOURCE_DIR` users should be served by specifying `-resource-dir=` in 
> a configuration file 
> (https://clang.llvm.org/docs/UsersManual.html#configuration-files)

I think I agree with you here.  The other problem I see with CLANG_RESOURCE_DIR 
is that it depends on the value of LLVM_LIBDIR_SUFFIX, so it's still 
configurable even without the CLANG_RESOURCE_DIR variable.  I think it would 
make sense to standardize it to /usr/lib/clang rather than 
/usr/lib${LLVM_LBDIR_SUFFIX}/clang.  This is how gcc works and it also ensures 
that clang can find the 32-bit compiler-rt libraries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[Lldb-commits] [PATCH] D152582: [lldb] Change return type of UnixSignals::GetShortName

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

In D152582#4410002 , @mib wrote:

> Don't you need to update the callers as well ? Otherwise LGTM.

ConstString can be implicitly converted to StringRef which is what's happening 
here. There are only 2 call sites, both pass in a ConstString.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152582

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

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 530084.
bulbazord added a comment.

- Converted some things to use multiline string literals
- Converted some f-strings into string literals when they didn't need 
interpolation
- Applied black formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

Files:
  lldb/scripts/generate-project.py

Index: lldb/scripts/generate-project.py
===
--- /dev/null
+++ lldb/scripts/generate-project.py
@@ -0,0 +1,304 @@
+#!/usr/bin/env python3
+
+# Project generation script
+# =
+# The purpose of this script is to generate a project with an arbitrary amount
+# of source files. This is useful for testing the performance impact of a given
+# LLDB change. For example, you can use this tool to generate a project with
+# 10,000 C++ source files and see how quickly LLDB processes the resulting
+# binary and related metadata (e.g. debug info).
+
+import os
+import sys
+import typing
+
+
+def print_usage() -> None:
+print("Usage: generate-project.py   ")
+
+
+def generate_c_header(directory: str, index: int) -> None:
+header_path = f"{directory}/obj{index}.h"
+with open(header_path, "w") as f:
+file_contents = (
+f"#ifndef _OBJ{index}_H\n"
+f"#define _OBJ{index}_H\n"
+f"extern int obj{index};\n"
+f"void call_obj{index}(void);\n"
+f"#endif // _OBJ{index}_H\n"
+)
+f.write(file_contents)
+
+
+def generate_c_impl(directory: str, index: int) -> None:
+impl_path = f"{directory}/obj{index}.c"
+with open(impl_path, "w") as f:
+file_contents = (
+"#include \n"
+f'#include "obj{index}.h"\n\n'
+f"int obj{index} = {index};\n"
+f"void call_obj{index}(void)"
+" {\n"
+f'  printf("%d\\n", obj{index});\n'
+"}\n"
+)
+f.write(file_contents)
+
+
+def generate_cpp_header(directory: str, index: int) -> None:
+header_path = f"{directory}/obj{index}.h"
+with open(header_path, "w") as f:
+file_contents = (
+f"#ifndef _OBJ{index}_H\n"
+f"#define _OBJ{index}_H\n"
+"namespace obj {\n"
+f"class Obj{index}"
+" {\n"
+"public:\n"
+"  static void Call(void);\n"
+"}; // class\n"
+"} // namespace\n"
+f"#endif // _OBJ{index}_H\n"
+)
+f.write(file_contents)
+
+
+def generate_cpp_impl(directory: str, index: int) -> None:
+impl_path = f"{directory}/obj{index}.cpp"
+with open(impl_path, "w") as f:
+file_contents = (
+"#include \n"
+f'#include "obj{index}.h"\n\n'
+"namespace obj {\n"
+f"void Obj{index}::Call(void)"
+" {\n"
+f"  std::cout << {index} << std::endl;;\n"
+"}\n"
+"} // namespace\n"
+)
+f.write(file_contents)
+
+
+def generate_swift_impl(directory: str, index: int) -> None:
+impl_path = f"{directory}/obj{index}.swift"
+with open(impl_path, "w") as f:
+file_contents = (
+f"public func call_obj{index}()" " {\n" f'  print("{index}")\n' "}\n"
+)
+f.write(file_contents)
+
+
+def generate_c_driver(directory: str, number_of_objects: int) -> None:
+main_path = f"{directory}/main.c"
+with open(main_path, "w") as f:
+for i in range(number_of_objects):
+f.write(f'#include "obj{i}.h"\n')
+
+f.write("int main() {\n")
+for i in range(number_of_objects):
+f.write(f"  call_obj{i}();\n")
+
+f.write("  return 0;\n")
+f.write("}\n")
+
+
+def generate_cpp_driver(directory: str, number_of_objects: int) -> None:
+main_path = f"{directory}/main.cpp"
+with open(main_path, "w") as f:
+for i in range(number_of_objects):
+f.write(f'#include "obj{i}.h"\n')
+
+f.write("using namespace obj;\n")
+f.write("int main() {\n")
+for i in range(number_of_objects):
+f.write(f"  Obj{i}::Call();\n")
+
+f.write("  return 0;\n")
+f.write("}\n")
+
+
+def generate_swift_driver(directory: str, number_of_objects: int) -> None:
+main_path = f"{directory}/main.swift"
+with open(main_path, "w") as f:
+for i in range(number_of_objects):
+f.write(f"import obj{i}\n")
+
+f.write("public func main() {\n")
+for i in range(number_of_objects):
+f.write(f"  call_obj{i}()\n")
+f.write("}\n")
+f.write("main()\n")
+
+
+def generate_c_makefile(directory: str, number_of_objects: int) -> None:
+makefile_path = f"{directory}/Makefile"
+with open(makefile_path, "w") as f:
+f.write("OBJDIR=objs\n\n")
+f.write("objects = \\\n")
+for

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Alex Langford via Phabricator via lldb-commits]

bulbazord marked 3 inline comments as done.
bulbazord added inline comments.



Comment at: lldb/scripts/generate-project.py:175
+# Building Objects and their respective swiftmodule
+f.write(f"$(OBJDIR)/%.o: %.swift objdir\n")
+f.write("\t$(SWIFT_FE) -c -primary-file $< -I objs/ \\\n")

kastiglione wrote:
> aprantl wrote:
> > Would `"""` multiline literals be more readable?
> +1
I changed it, what do y'all think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

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

[Lldb-commits] [PATCH] D152560: Make TestStepNodebug more robust against codegen changes

[Adrian Prantl via Phabricator via lldb-commits]

aprantl abandoned this revision.
aprantl added a comment.

According to an offline conversation with @jingham this is still not the 
expected behavior from the stepping engine.


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

https://reviews.llvm.org/D152560

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

[Lldb-commits] [PATCH] D152569: [lldb] Introduce a tool to quickly generate projects with an arbitrary number of sources

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: lldb/scripts/generate-project.py:23
+with open(header_path, "w") as f:
+file_contents = (
+f"#ifndef _OBJ{index}_H\n"

nit: Is it necessary to put it in a variable or could you have a multiline 
string in the `write` call ?



Comment at: lldb/scripts/generate-project.py:183-190
+f.write("OBJDIR=objs\n")
+f.write("SDK=$(shell xcrun --show-sdk-path)\n")
+f.write("SWIFT_FE=swift -frontend\n")
+f.write("SWIFT_FEFLAGS=-g -Onone -serialize-debugging-options \\\n")
+f.write("  -sdk $(SDK) -enable-anonymous-context-mangled-names\n")
+f.write("SWIFTC=swiftc\n")
+f.write("\n")

What about here ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152569

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

[Lldb-commits] [PATCH] D152582: [lldb] Change return type of UnixSignals::GetShortName

[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/D152582/new/

https://reviews.llvm.org/D152582

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

[Lldb-commits] [PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D151683#4409633 , @aaron.ballman 
wrote:

> In D151683#4384017 , @erichkeane 
> wrote:
>
>> In D151683#4382408 , @philnik 
>> wrote:
>>
>>> No. I guess, since you are asking I should write one for this? Only for the 
>>> removal of `-fdouble-square-bracket-attributes`, or also for back porting 
>>> this?
>>
>> The RFC I would expect for "allow C/C++ style attributes as an extension in 
>> previous modes".  This is a pretty significant feature to allow as an 
>> extension, particularly since our two main alternative compilers (G++/MSVC) 
>> don't have this as an extension. I'd be curious how the other C compilers do 
>> this as well.
>
> I think this does warrant an RFC because it impacts both C++ and C, but I'm 
> hoping the RFC is uncontroversial. It's mostly to establish what the use 
> cases are for needing the extension. The feature is conforming as an 
> extension to both languages, and I don't know of any breakage that would come 
> from enabling it by default. I'm not certain whether we want to remove the 
> feature flag immediately or whether we'd like to give one release of warning 
> about it being removed (I'll defer to whatever @MaskRay thinks is reasonable) 
> -- that search is compelling for why it's safe to remove the option, but 
> there's plenty of proprietary code which we don't know about, so it's 
> possible the option is still used. I'd be especially curious to know if 
> anyone is using `-fno-double-square-bracket-attributes` to disable the 
> feature in a mode where it would otherwise be enabled. I'm adding the 
> `clang-vendors` group as a subscriber as an early warning that removing the 
> command line option could be a potentially breaking change.
>
> In terms of implementation work, there's still some minor stuff to address. 
> Also, please be sure to also add a release note about the changes, and a 
> potentially breaking entry for removing the command line option (assuming we 
> elect to go that route).

If allowing the extension in older language modes deems good, removing 
`-fno-double-square-bracket-attributes` seems fine if (and thanks for CCing the 
`clang-vendors` group).
If we want to play safely, we can default `-fdouble-square-bracket-attributes` 
to true in this patch and remove the option in a follow-up change possibly 
after some time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151683

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

[Lldb-commits] [PATCH] D152590: Streamline expression parser error messages

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, kastiglione, JDevlieghere.
Herald added a project: All.
aprantl requested review of this revision.

Currently the expression parser prints a mostly useless generic error before 
printing the compiler error:

  (lldb) p 1+x)
  error: expression failed to parse:
  error: :1:3: use of undeclared identifier 'x'
  1+x)
^

This is distracting and as far as I can tell only exists to work around the 
fact that the first "error: " is unconditionally injected by 
`CommandReturnObject`. The solution is not very elegant, but the result looks 
much better.

(Partially addresses rdar://110492710)


https://reviews.llvm.org/D152590

Files:
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
  lldb/test/API/commands/expression/fixits/TestFixIts.py


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -154,7 +154,6 @@
 multiple_runs_options.SetRetriesWithFixIts(0)
 value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
 errmsg = value.GetError().GetCString()
-self.assertIn("expression failed to parse", errmsg)
 self.assertIn("using declaration resolved to type without 'typename'", 
errmsg)
 self.assertIn("fixed expression suggested:", errmsg)
 self.assertIn("using typename T::TypeDef", errmsg)
@@ -166,7 +165,6 @@
 multiple_runs_options.SetRetriesWithFixIts(1)
 value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
 errmsg = value.GetError().GetCString()
-self.assertIn("expression failed to parse", errmsg)
 self.assertIn("fixed expression suggested:", errmsg)
 # Both our fixed expressions should be in the suggested expression.
 self.assertIn("using typename T::TypeDef", errmsg)
Index: lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
===
--- lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -151,8 +151,7 @@
 value = frame.EvaluateExpression("struct Redef { float y; };", 
top_level_opts)
 self.assertFalse(value.GetError().Success())
 self.assertIn(
-"""
-error: :1:8: redefinition of 'Redef'
+"""error: :1:8: redefinition of 'Redef'
 1 | struct Redef { float y; };
   |^
 :1:8: previous definition is here
Index: lldb/source/Interpreter/CommandReturnObject.cpp
===
--- lldb/source/Interpreter/CommandReturnObject.cpp
+++ lldb/source/Interpreter/CommandReturnObject.cpp
@@ -101,7 +101,10 @@
   SetStatus(eReturnStatusFailed);
   if (in_string.empty())
 return;
-  error(GetErrorStream()) << in_string.rtrim() << '\n';
+  // Workaround to deal with already fully formatted compiler diagnostics.
+  llvm::StringRef msg(in_string.rtrim());
+  msg.consume_front("error: ");
+  error(GetErrorStream()) << msg << '\n';
 }
 
 void CommandReturnObject::SetError(const Status &error,
Index: lldb/source/Expression/UserExpression.cpp
===
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -330,11 +330,10 @@
   std::string msg;
   {
 llvm::raw_string_ostream os(msg);
-os << "expression failed to parse:\n";
 if (!diagnostic_manager.Diagnostics().empty())
   os << diagnostic_manager.GetString();
 else
-  os << "unknown error";
+  os << "expression failed to parse (no further compiler diagnostics)";
 if (target->GetEnableNotifyAboutFixIts() && fixed_expression &&
 !fixed_expression->empty())
   os << "\nfixed expression suggested:\n  " << *fixed_expression;


Index: lldb/test/API/commands/expression/fixits/TestFixIts.py
===
--- lldb/test/API/commands/expression/fixits/TestFixIts.py
+++ lldb/test/API/commands/expression/fixits/TestFixIts.py
@@ -154,7 +154,6 @@
 multiple_runs_options.SetRetriesWithFixIts(0)
 value = frame.EvaluateExpression(two_runs_expr, multiple_runs_options)
 errmsg = value.GetError().GetCString()
-self.assertIn("expression failed to parse", errmsg)
 self.assertIn("using declaration resolved to type without 'typename'", errmsg)
 self.assertIn("fixed expression suggested:", errmsg)
 self.assertIn("using typename T::TypeDef", errmsg)
@@ -166,7 +165,6 @@
 multiple_runs_options.SetRetriesWithFixIts(1)

[Lldb-commits] [lldb] aff35b2 - [lldb][NFCI] Replace some static ConstStrings with StringLiterals in Disassembler

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-06-09T15:38:48-07:00
New Revision: aff35b2f1ef5de1ec5c40e5dca3b88c556e03554

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

LOG: [lldb][NFCI] Replace some static ConstStrings with StringLiterals in 
Disassembler

These should have been replaced in e53e1de57ecc but I missed them
because ConstStrings can be implicitly converted to llvm::StringRefs.

Added: 


Modified: 
lldb/source/Core/Disassembler.cpp

Removed: 




diff  --git a/lldb/source/Core/Disassembler.cpp 
b/lldb/source/Core/Disassembler.cpp
index 09eee082bc394..5e79aa40cb33e 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -893,8 +893,8 @@ bool Instruction::TestEmulation(Stream *out_stream, const 
char *file_name) {
 
   OptionValueDictionary *data_dictionary =
   data_dictionary_sp->GetAsDictionary();
-  static ConstString description_key("assembly_string");
-  static ConstString triple_key("triple");
+  static constexpr llvm::StringLiteral description_key("assembly_string");
+  static constexpr llvm::StringLiteral triple_key("triple");
 
   OptionValueSP value_sp = data_dictionary->GetValueForKey(description_key);
 



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

[Lldb-commits] [PATCH] D152594: [lldb] Introduce DynamicRegisterInfo::CreateFromDict

[Alex Langford via Phabricator via lldb-commits]

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

I want to add some error handling to DynamicRegisterInfo because there
are many operations that can fail and many of these operations do not
give meaningful information back to the caller.

To begin that process, I want to add a static method that is responsible
for creating a DynamicRegisterInfo from a StructuredData::Dictionary
(and ArchSpec). This is meant to replace the equivalent constructor
because constructors are ill-equipped to perform error handling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152594

Files:
  lldb/include/lldb/Target/DynamicRegisterInfo.h
  lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -20,10 +20,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-DynamicRegisterInfo::DynamicRegisterInfo(
-const lldb_private::StructuredData::Dictionary &dict,
-const lldb_private::ArchSpec &arch) {
-  SetRegisterInfo(dict, arch);
+std::unique_ptr
+DynamicRegisterInfo::CreateFromDict(const StructuredData::Dictionary &dict,
+const ArchSpec &arch) {
+  auto dyn_reg_info = std::make_unique();
+  if (!dyn_reg_info)
+return nullptr;
+
+  if (dyn_reg_info->SetRegisterInfo(dict, arch) == 0)
+return nullptr;
+
+  return dyn_reg_info;
 }
 
 DynamicRegisterInfo::DynamicRegisterInfo(DynamicRegisterInfo &&info) {
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -341,7 +341,7 @@
   LLVM_PRETTY_FUNCTION, "Failed to get scripted thread registers 
info.",
   error, LLDBLog::Thread);
 
-m_register_info_sp = std::make_shared(
+m_register_info_sp = DynamicRegisterInfo::CreateFromDict(
 *reg_info, m_scripted_process.GetTarget().GetArchitecture());
   }
 
Index: lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
===
--- lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -128,8 +128,9 @@
 if (!dictionary)
   return nullptr;
 
-m_register_info_up = std::make_unique(
+m_register_info_up = DynamicRegisterInfo::CreateFromDict(
 *dictionary, m_process->GetTarget().GetArchitecture());
+assert(m_register_info_up);
 assert(m_register_info_up->GetNumRegisters() > 0);
 assert(m_register_info_up->GetNumRegisterSets() > 0);
   }
Index: lldb/include/lldb/Target/DynamicRegisterInfo.h
===
--- lldb/include/lldb/Target/DynamicRegisterInfo.h
+++ lldb/include/lldb/Target/DynamicRegisterInfo.h
@@ -46,8 +46,8 @@
 
   DynamicRegisterInfo() = default;
 
-  DynamicRegisterInfo(const lldb_private::StructuredData::Dictionary &dict,
-  const lldb_private::ArchSpec &arch);
+  static std::unique_ptr
+  CreateFromDict(const StructuredData::Dictionary &dict, const ArchSpec &arch);
 
   virtual ~DynamicRegisterInfo() = default;
 


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -20,10 +20,17 @@
 using namespace lldb;
 using namespace lldb_private;
 
-DynamicRegisterInfo::DynamicRegisterInfo(
-const lldb_private::StructuredData::Dictionary &dict,
-const lldb_private::ArchSpec &arch) {
-  SetRegisterInfo(dict, arch);
+std::unique_ptr
+DynamicRegisterInfo::CreateFromDict(const StructuredData::Dictionary &dict,
+const ArchSpec &arch) {
+  auto dyn_reg_info = std::make_unique();
+  if (!dyn_reg_info)
+return nullptr;
+
+  if (dyn_reg_info->SetRegisterInfo(dict, arch) == 0)
+return nullptr;
+
+  return dyn_reg_info;
 }
 
 DynamicRegisterInfo::DynamicRegisterInfo(DynamicRegisterInfo &&info) {
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -341,7 +341,7 @@
   LLVM_PRETTY_FUNCTION, "Failed to get scripted thread registers info.",

[Lldb-commits] [PATCH] D152597: [lldb][NFCI] Remove StructuredData::Dictionary::GetValueForKeyAsString overloads involving ConstString

[Alex Langford via Phabricator via lldb-commits]

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

In an effort to keep the ConstString StringPool small, I plan on
removing use of ConstString in StructuredData. The only class that
really uses it is StructuredData::Dictionary.

This one was fairly easy to remove, I plan on removing the others in
follow-up changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152597

Files:
  lldb/include/lldb/Utility/StructuredData.h
  lldb/source/Target/DynamicRegisterInfo.cpp


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -238,17 +238,20 @@
 std::vector invalidate_regs;
 memset(®_info, 0, sizeof(reg_info));
 
-ConstString name_val;
-ConstString alt_name_val;
-if (!reg_info_dict->GetValueForKeyAsString("name", name_val, nullptr)) {
+llvm::StringRef name_val;
+if (!reg_info_dict->GetValueForKeyAsString("name", name_val)) {
   Clear();
   printf("error: registers must have valid names and offsets\n");
   reg_info_dict->DumpToStdout();
   return 0;
 }
-reg_info.name = name_val.GetCString();
-reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val, nullptr);
-reg_info.alt_name = alt_name_val.GetCString();
+reg_info.name = ConstString(name_val).GetCString();
+
+llvm::StringRef alt_name_val;
+if (reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val))
+  reg_info.alt_name = ConstString(alt_name_val).GetCString();
+else
+  reg_info.alt_name = nullptr;
 
 llvm::Expected byte_offset =
 ByteOffsetFromRegInfoDict(i, *reg_info_dict, byte_order);
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -536,26 +536,6 @@
   return success;
 }
 
-bool GetValueForKeyAsString(llvm::StringRef key,
-ConstString &result) const {
-  ObjectSP value_sp = GetValueForKey(key);
-  if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
-  result = ConstString(string_value->GetValue());
-  return true;
-}
-  }
-  return false;
-}
-
-bool GetValueForKeyAsString(llvm::StringRef key, ConstString &result,
-const char *default_val) const {
-  bool success = GetValueForKeyAsString(key, result);
-  if (!success)
-result.SetCString(default_val);
-  return success;
-}
-
 bool GetValueForKeyAsDictionary(llvm::StringRef key,
 Dictionary *&result) const {
   result = nullptr;


Index: lldb/source/Target/DynamicRegisterInfo.cpp
===
--- lldb/source/Target/DynamicRegisterInfo.cpp
+++ lldb/source/Target/DynamicRegisterInfo.cpp
@@ -238,17 +238,20 @@
 std::vector invalidate_regs;
 memset(®_info, 0, sizeof(reg_info));
 
-ConstString name_val;
-ConstString alt_name_val;
-if (!reg_info_dict->GetValueForKeyAsString("name", name_val, nullptr)) {
+llvm::StringRef name_val;
+if (!reg_info_dict->GetValueForKeyAsString("name", name_val)) {
   Clear();
   printf("error: registers must have valid names and offsets\n");
   reg_info_dict->DumpToStdout();
   return 0;
 }
-reg_info.name = name_val.GetCString();
-reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val, nullptr);
-reg_info.alt_name = alt_name_val.GetCString();
+reg_info.name = ConstString(name_val).GetCString();
+
+llvm::StringRef alt_name_val;
+if (reg_info_dict->GetValueForKeyAsString("alt-name", alt_name_val))
+  reg_info.alt_name = ConstString(alt_name_val).GetCString();
+else
+  reg_info.alt_name = nullptr;
 
 llvm::Expected byte_offset =
 ByteOffsetFromRegInfoDict(i, *reg_info_dict, byte_order);
Index: lldb/include/lldb/Utility/StructuredData.h
===
--- lldb/include/lldb/Utility/StructuredData.h
+++ lldb/include/lldb/Utility/StructuredData.h
@@ -536,26 +536,6 @@
   return success;
 }
 
-bool GetValueForKeyAsString(llvm::StringRef key,
-ConstString &result) const {
-  ObjectSP value_sp = GetValueForKey(key);
-  if (value_sp.get()) {
-if (auto string_value = value_sp->GetAsString()) {
-  result = ConstString(string_value->GetValue());
-  return true;
-}
-  }
-  return false;
-}
-
-bool GetValueForKeyAsString(llvm::StringRef key, C

[Lldb-commits] [PATCH] D152594: [lldb] Introduce DynamicRegisterInfo::CreateFromDict

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

Are we replacing all the constructors that can fail by static methods ? 
Wouldn't it be easier to pass an `Status&` as an extra parameter ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152594

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

[Lldb-commits] [PATCH] D152597: [lldb][NFCI] Remove StructuredData::Dictionary::GetValueForKeyAsString overloads involving ConstString

[Med Ismail Bennani via Phabricator via lldb-commits]

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

I thought `StructuredData::String` used a `ConstString` for storage but it 
actually uses a `std::string` instead. Makes sense to remove this then. LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152597

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