[LLDB] Switch to using DIL as default implementation for 'frame var'.

[cmtice]

No description provided.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (cmtice)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/147887.diff

2 Files Affected:

• (modified) lldb/source/Target/TargetProperties.td (+2-2)
• (modified) llvm/docs/ReleaseNotes.md (+8)

diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index 4aa9e046d6077..656503bb8d228 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -5,8 +5,8 @@ let Definition = "target_experimental" in {
     Global, DefaultTrue,
     Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
   def UseDIL : Property<"use-DIL", "Boolean">,
-    Global, DefaultFalse,
-    Desc<"If true, use the alternative DIL implementation for frame variable evaluation.">;
+    Global, DefaultTrue,
+    Desc<"If true, use the DIL implementation for frame variable evaluation.">;
 }
 
 let Definition = "target" in {
diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md
index daf822388a2ff..5d2146b7f2f75 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -306,6 +306,14 @@ Changes to LLDB
     stop reason = SIGSEGV: sent by tkill system call (sender pid=649752, uid=2667987)
   ```
 * ELF Cores can now have their siginfo structures inspected using `thread siginfo`.
+* LLDB now uses
+  [DIL](https://discourse.llvm.org/t/rfc-data-inspection-language/69893) as the
+  default implementation for 'frame variable'. This should not change the
+  behavior of 'frame variable' at all, at this time. To revert to using the
+  old implementation use
+  ```
+     settings set target.experimental.use-DIL false
+   ```
 
 ### Changes to lldb-dap
 

[labath]

There are a couple of failures in the CI. The backtraces don't make a whole lot sense, but it looks like there's something wrong with retrieving the list of variables from a CU.

[kuilpd]

I addressed lldb-shell-subprocess tests failing in #147955 , but there's still 2 data formatter tests failing, not sure what's happening there yet.

[Michael137]

but there's still 2 data formatter tests failing, not sure what's happening there yet.

FWIW, the object child of a std::shared_ptr should be equivalent to dereferencing it. How does DIL handle the $$dereference$$ operator? Does it do anything special?

[cmtice]

but there's still 2 data formatter tests failing, not sure what's happening there yet.

FWIW, the object child of a std::shared_ptr should be equivalent to dereferencing it. How does DIL handle the $$dereference$$ operator? Does it do anything special?

DIL handles it exactly the same way the current 'frame variable' handles it, namely it uses the provided synthetic children and data formatters. (Gets the synthetic value for the shared pointer, then calls 'GetChildMemberWithName("object")', which returns the correct thing.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[cmtice]

I think this PR is ready to be reviewed again. Please take a look when you have a few minutes. :-)

[labath]

I gave this patch a spin on a mac. The only failure I got was in test/Shell/SymbolFile/DWARF/TestDedupWarnings.test, which fails it does not generate both of the warnings, and that's because the "p" (aka dwim-print) now resolves to "frame var" instead of expr". Changing the test to use "expr a/b" instead should fix it.

[cmtice]

I gave this patch a spin on a mac. The only failure I got was in test/Shell/SymbolFile/DWARF/TestDedupWarnings.test, which fails it does not generate both of the warnings, and that's because the "p" (aka dwim-print) now resolves to "frame var" instead of expr". Changing the test to use "expr a/b" instead should fix it.

Done.

[labath]

🤞

[labath]

It looks like this just barely didn't make it into the 21.x release branch. Do we want to cherry-pick that over? I think it would be a good way to flush out any issues...

[DavidSpickett]

Perhaps with a release note saying this happened and how to switch back to the non-default way?

[cmtice]

Perhaps with a release note saying this happened and how to switch back to the non-default way?

This PR updates the release notes; is that not good enough?

[DavidSpickett]

Add a : on the end here, or put the command on the same line, so it's one sentence.

[DavidSpickett]

This PR updates the release notes; is that not good enough?

Yes that's perfect, didn't see that it was already there.

[kuilpd]

It looks like this just barely didn't make it into the 21.x release branch. Do we want to cherry-pick that over? I think it would be a good way to flush out any issues...

I think so too, this and #149117. Do we just create a PR with the same changes into release/21.x branch?

[labath]

The process for that is here, though it's a bit unfortunate that a week has passed since then (I was OOO this week so I couldn't reply sooner). At this point, I'm no longer sure it makes sense to do that.

[kuilpd]

The process for that is here, though it's a bit unfortunate that a week has passed since then (I was OOO this week so I couldn't reply sooner). At this point, I'm no longer sure it makes sense to do that.

We could still do it before the next minor release, at least I feel like it should be.

[labath]

In that case, please create the cherry pick as soon as possible, and we can see where that discussion leads us. For myself, I'm going to defer @JDevlieghere opinion.

[JDevlieghere]

In that case, please create the cherry pick as soon as possible, and we can see where that discussion leads us. For myself, I'm going to defer @JDevlieghere opinion.

I'm supportive. For Swift we'll be basing our release branch on the LLVM one and I'd like us to start living on it there as well.

[labath]

/cherry-pick f5c676d 33396d7

[labath]

Okay, here goes nothing.

[llvmbot]

/pull-request #150600