teemperor updated this revision to Diff 196965.
teemperor added a comment.

- Disable the variable filtering when during tab completion for now, as it 
seems our Clang lookup doesn't pull in external types which breaks the 
completion tests (and others too). This is only a temporary fix to unblock 
D59960 <https://reviews.llvm.org/D59960>.
- Some minor cleanup and documentation.


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

https://reviews.llvm.org/D46551

Files:
  
lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
@@ -178,11 +178,12 @@
   std::vector<std::string> GetModulesToImport(ExecutionContext &exe_ctx);
   void UpdateLanguageForExpr(DiagnosticManager &diagnostic_manager,
                              ExecutionContext &exe_ctx,
-                             std::vector<std::string> modules_to_import);
+                             std::vector<std::string> modules_to_import,
+                             bool for_completion);
   bool SetupPersistentState(DiagnosticManager &diagnostic_manager,
                                    ExecutionContext &exe_ctx);
   bool PrepareForParsing(DiagnosticManager &diagnostic_manager,
-                         ExecutionContext &exe_ctx);
+                         ExecutionContext &exe_ctx, bool for_completion);
 
   ClangUserExpressionHelper m_type_system_helper;
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -386,7 +386,7 @@
 
 void ClangUserExpression::UpdateLanguageForExpr(
     DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
-    std::vector<std::string> modules_to_import) {
+    std::vector<std::string> modules_to_import, bool for_completion) {
   m_expr_lang = lldb::LanguageType::eLanguageTypeUnknown;
 
   std::string prefix = m_expr_prefix;
@@ -407,7 +407,7 @@
 
     if (!source_code->GetText(m_transformed_text, m_expr_lang,
                               m_in_static_method, exe_ctx, !m_ctx_obj,
-                              modules_to_import)) {
+                              for_completion, modules_to_import)) {
       diagnostic_manager.PutString(eDiagnosticSeverityError,
                                    "couldn't construct expression body");
       return;
@@ -482,7 +482,8 @@
 }
 
 bool ClangUserExpression::PrepareForParsing(
-    DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) {
+    DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx,
+    bool for_completion) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   InstallContext(exe_ctx);
@@ -511,7 +512,8 @@
   LLDB_LOG(log, "List of imported modules in expression: {0}",
            llvm::make_range(used_modules.begin(), used_modules.end()));
 
-  UpdateLanguageForExpr(diagnostic_manager, exe_ctx, used_modules);
+  UpdateLanguageForExpr(diagnostic_manager, exe_ctx, used_modules,
+                        for_completion);
   return true;
 }
 
@@ -522,7 +524,7 @@
                                 bool generate_debug_info) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
-  if (!PrepareForParsing(diagnostic_manager, exe_ctx))
+  if (!PrepareForParsing(diagnostic_manager, exe_ctx, /*for_completion*/ false))
     return false;
 
   if (log)
@@ -721,7 +723,7 @@
   // correct.
   DiagnosticManager diagnostic_manager;
 
-  if (!PrepareForParsing(diagnostic_manager, exe_ctx))
+  if (!PrepareForParsing(diagnostic_manager, exe_ctx, /*for_completion*/ true))
     return false;
 
   if (log)
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
@@ -43,13 +43,14 @@
   ///        evaluated.
   /// \param add_locals True iff local variables should be injected into the
   ///        expression source code.
+  /// \param force_add_all_locals True iff all local variables should be
+  ///        injected even if they are not used in the expression.
   /// \param modules A list of (C++) modules that the expression should import.
   ///
   /// \return true iff the source code was successfully generated.
   bool GetText(std::string &text, lldb::LanguageType wrapping_language,
-               bool static_method,
-               ExecutionContext &exe_ctx,
-               bool add_locals,
+               bool static_method, ExecutionContext &exe_ctx, bool add_locals,
+               bool force_add_all_locals,
                llvm::ArrayRef<std::string> modules) const;
 
   // Given a string returned by GetText, find the beginning and end of the body
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -8,6 +8,9 @@
 
 #include "ClangExpressionSourceCode.h"
 
+#include "clang/Basic/CharInfo.h"
+#include "llvm/ADT/StringRef.h"
+
 #include "Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.h"
 #include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "lldb/Symbol/Block.h"
@@ -161,8 +164,43 @@
   }
 }
 
+/// Checks if the expression body contains the given variable as a token.
+/// \param body The expression body.
+/// \param var The variable token we are looking for.
+/// \return True iff the expression body containes the variable as a token.
+static bool ExprBodyContainsVar(llvm::StringRef body, llvm::StringRef var) {
+  assert(var.find_if([](char c) { return !clang::isIdentifierBody(c); }) ==
+             llvm::StringRef::npos &&
+         "variable contains non-identifier chars?");
+
+  size_t start = 0;
+  // Iterate over all occurences of the variable string in our expression.
+  while ((start = body.find(var, start)) != llvm::StringRef::npos) {
+    // We found our variable name in the expression. Check that the token
+    // that contains our needle is equal to our variable and not just contains
+    // the character sequence by accident.
+    // Prevents situations where we for example inlcude the variable 'FOO' in an
+    // expression like 'FOObar + 1'.
+    bool has_characters_before =
+        start != 0 && clang::isIdentifierBody(body[start - 1]);
+    bool has_characters_after =
+        start + var.size() < body.size() &&
+        clang::isIdentifierBody(body[start + var.size()]);
+
+    // Our token just contained the variable name as a substring. Continue
+    // searching the rest of the expression.
+    if (has_characters_before || has_characters_after) {
+      ++start;
+      continue;
+    }
+    return true;
+  }
+  return false;
+}
+
 static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
-                                  StreamString &stream) {
+                                  StreamString &stream,
+                                  const std::string &expr) {
   for (size_t i = 0; i < var_list_sp->GetSize(); i++) {
     lldb::VariableSP var_sp = var_list_sp->GetVariableAtIndex(i);
 
@@ -170,15 +208,17 @@
     if (!var_name || var_name == "this" || var_name == ".block_descriptor")
       continue;
 
+    if (!expr.empty() && !ExprBodyContainsVar(expr, var_name.GetStringRef()))
+      continue;
+
     stream.Printf("using $__lldb_local_vars::%s;\n", var_name.AsCString());
   }
 }
 
-bool ClangExpressionSourceCode::GetText(std::string &text,
-                                   lldb::LanguageType wrapping_language,
-                                   bool static_method,
-                                   ExecutionContext &exe_ctx, bool add_locals,
-                                   llvm::ArrayRef<std::string> modules) const {
+bool ClangExpressionSourceCode::GetText(
+    std::string &text, lldb::LanguageType wrapping_language, bool static_method,
+    ExecutionContext &exe_ctx, bool add_locals, bool force_add_all_locals,
+    llvm::ArrayRef<std::string> modules) const {
   const char *target_specific_defines = "typedef signed char BOOL;\n";
   std::string module_macros;
 
@@ -256,7 +296,8 @@
         if (target->GetInjectLocalVariables(&exe_ctx)) {
           lldb::VariableListSP var_list_sp =
               frame->GetInScopeVariableList(false, true);
-          AddLocalVariableDecls(var_list_sp, lldb_local_var_decls);
+          AddLocalVariableDecls(var_list_sp, lldb_local_var_decls,
+                                force_add_all_locals ? "" : m_body);
         }
       }
     }
Index: lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
+++ lldb/packages/Python/lldbsuite/test/lang/cpp/member-and-local-vars-with-same-name/TestMembersAndLocalsWithSameName.py
@@ -155,7 +155,9 @@
         frame = thread.GetSelectedFrame()
         self.assertTrue(frame.IsValid())
 
+        self.enable_expression_log()
         val = frame.EvaluateExpression("a")
+        self.disable_expression_log_and_check_for_locals(['a'])
         self.assertTrue(val.IsValid())
         self.assertEqual(val.GetValueAsUnsigned(), 12345)
 
@@ -189,6 +191,12 @@
         self.assertTrue(val.IsValid())
         self.assertEqual(val.GetValueAsUnsigned(), 10003)
 
+        self.enable_expression_log()
+        val = frame.EvaluateExpression("c-b")
+        self.disable_expression_log_and_check_for_locals(['c','b'])
+        self.assertTrue(val.IsValid())
+        self.assertEqual(val.GetValueAsUnsigned(), 1)
+
         self.process.Continue()
         self.assertTrue(
             self.process.GetState() == lldb.eStateStopped,
@@ -211,6 +219,13 @@
         self.assertTrue(val.IsValid())
         self.assertEqual(val.GetValueAsUnsigned(), 778899)
 
+        self.enable_expression_log()
+        val = frame.EvaluateExpression("a+b")
+        self.disable_expression_log_and_check_for_locals(['a','b'])
+        self.assertTrue(val.IsValid())
+        self.assertEqual(val.GetValueAsUnsigned(), 3)
+
+
     def _load_exe(self):
         self.build()
 
@@ -234,7 +249,9 @@
         frame = thread.GetSelectedFrame()
         self.assertTrue(frame.IsValid())
 
+        self.enable_expression_log()
         val = frame.EvaluateExpression("a")
+        self.disable_expression_log_and_check_for_locals([])
         self.assertTrue(val.IsValid())
         self.assertEqual(val.GetValueAsUnsigned(), 112233)
 
@@ -245,3 +262,23 @@
         val = frame.EvaluateExpression("c")
         self.assertTrue(val.IsValid())
         self.assertEqual(val.GetValueAsUnsigned(), 778899)
+
+    def enable_expression_log(self):
+        log_file = os.path.join(self.getBuildDir(), "expr.log")
+        self.runCmd("log enable  -f '%s' lldb expr" % (log_file))
+
+    def disable_expression_log_and_check_for_locals(self, variables):
+        log_file = os.path.join(self.getBuildDir(), "expr.log")
+        self.runCmd("log disable lldb expr")
+        local_var_regex = re.compile(r".*__lldb_local_vars::(.*);")
+        matched = []
+        with open(log_file, 'r') as log:
+            for line in log:
+                if line.find('LLDB_BODY_START') != -1:
+                    break
+                m = re.match(local_var_regex, line)
+                if m:
+                    self.assertIn(m.group(1), variables)
+                    matched.append(m.group(1))
+        self.assertEqual([item for item in variables if item not in matched],
+                         [])
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to