[Lldb-commits] [lldb] [LLDB] Add DIL code for handling plain variable names. (PR #120971)

[Pavel Labath via lldb-commits]

================
@@ -0,0 +1,191 @@
+//===-- DILLexer.cpp ------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/ValueObject/DILLexer.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+const std::string DILToken::getTokenName(dil::TokenKind kind) {
+  std::string retval;
+  switch (kind) {
+  case dil::TokenKind::coloncolon:
+    retval = "coloncolon";
+    break;
+  case dil::TokenKind::eof:
+    retval = "eof";
+    break;
+  case dil::TokenKind::identifier:
+    retval = "identifier";
+    break;
+  case dil::TokenKind::kw_namespace:
+    retval = "namespace";
+    break;
+  case dil::TokenKind::kw_this:
+    retval = "this";
+    break;
+  case dil::TokenKind::l_paren:
+    retval = "l_paren";
+    break;
+  case dil::TokenKind::r_paren:
+    retval = "r_paren";
+    break;
+  case dil::TokenKind::unknown:
+    retval = "unknown";
+    break;
+  default:
+    retval = "token_name";
+    break;
+  }
+  return retval;
+}
+
+static bool Is_Letter(char c) {
+  if (('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'))
+    return true;
+  return false;
+}
+
+static bool Is_Digit(char c) { return ('0' <= c && c <= '9'); }
+
+bool DILLexer::Is_Word(std::string::iterator start, uint32_t &length) {
+  bool done = false;
+  for (; m_cur_pos != m_expr.end() && !done; ++m_cur_pos) {
+    char c = *m_cur_pos;
+    if (!Is_Letter(c) && !Is_Digit(c) && c != '_') {
+      done = true;
+      break;
+    } else
+      length++;
+  }
+  if (length > 0)
+    return true;
+  else
+    m_cur_pos = start;
+  return false;
+}
+
+void DILLexer::UpdateLexedTokens(DILToken &result, dil::TokenKind tok_kind,
+                                 std::string tok_str, uint32_t tok_pos,
+                                 uint32_t tok_len) {
+  DILToken new_token;
+  result.setValues(tok_kind, tok_str, tok_pos, tok_len);
+  new_token = result;
+  m_lexed_tokens.push_back(std::move(new_token));
+}
+
+bool DILLexer::Lex(DILToken &result, bool look_ahead) {
+  bool retval = true;
+
+  if (!look_ahead) {
+    // We're being asked for the 'next' token, and not a part of a LookAhead.
+    // Check to see if we've already lexed it and pushed it onto our tokens
+    // vector; if so, return the next token from the vector, rather than doing
+    // more lexing.
+    if ((m_tokens_idx != UINT_MAX) &&
+        (m_tokens_idx < m_lexed_tokens.size() - 1)) {
+      result = m_lexed_tokens[m_tokens_idx + 1];
+      return retval;
+    }
+  }
+
+  // Skip over whitespace (spaces).
+  while (m_cur_pos != m_expr.end() && *m_cur_pos == ' ')
+    m_cur_pos++;
+
+  // Check to see if we've reached the end of our input string.
+  if (m_cur_pos == m_expr.end()) {
+    UpdateLexedTokens(result, dil::TokenKind::eof, "", m_expr.length(), 0);
+    return retval;
+  }
+
+  uint32_t position = m_cur_pos - m_expr.begin();
+  ;
+  std::string::iterator start = m_cur_pos;
+  uint32_t length = 0;
+  if (Is_Word(start, length)) {
+    dil::TokenKind kind;
+    std::string word = m_expr.substr(position, length);
+    if (word == "this")
----------------
labath wrote:

I don't think handling `self` as well would make it better.

The question of modifying `this` is interesting, but I don't think it should require special handling here. As far as modification is concerned, I think `this` is not different from a variable of type (e.g.) `const int` -- the language forbids you from modifying it, the debugger can do that anyway if it really wants to, but the result may not be what you expect:
```
Process 48439 stopped
* thread #1, name = 'a.out', stop reason = step over
    frame #0: 0x000055555555520b a.out`X::foo(this=0x00007fffffffd7d7) at a.cc:7:5
   4   	public:
   5   	  void foo() {
   6   	    const int x = 47;
-> 7   	    printf("this = %p, x = %d\n", this, x);
   8   	  }
   9   	};
   10  	
(lldb) v this x
(X *) this = 0x00007fffffffd7d7
(const int) x = 47
(lldb) script lldb.frame.FindVariable("x").SetValueFromCString("74")
True
(lldb) script lldb.frame.FindVariable("this").SetValueFromCString("0")
True
(lldb) v this x
(X *) this = nullptr
(const int) x = 74
(lldb) c
Process 48439 resuming
this = 0x7fffffffd7d7, x = 47
Process 48439 exited with status = 0 (0x00000000) 
```

Given that SBValue currently allows you to modify both variables, I think DIL (which is basically a domain-specific language for SBValue operations) should be doing the same. If that doesn't convince you, consider also this:
- gdb also allows you to modify `this`:
```
(gdb) set this = 0
(gdb) p this
$2 = (X *) 0x0
```
- if I compile the same binary with gcc, then the above lldb commands actually work (as in, the debugged binary "sees" the modified values of the two variables)

https://github.com/llvm/llvm-project/pull/120971