teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added subscribers: lldb-commits, abidh.
Herald added a project: LLDB.
teemperor edited the summary of this revision.

Currently if you evaluate for example "expression int" the user just sees 
nothing and has no way of knowing what
exactly could have gone wrong. This patch adds a `-w` flag to the expression 
command that causes compiler warnings
to be emitted even if the command succeeds. This was the agreed workaround to 
solve rdar://12788008.

The checkerror flag that was added to lldbtest was necessary as we usually 
don't check the error output of commands
when they succeed (but we have to do now).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D66739

Files:
  lldb/include/lldb/Expression/UserExpression.h
  lldb/include/lldb/Target/Target.h
  lldb/packages/Python/lldbsuite/test/expression_command/warnings/Makefile
  
lldb/packages/Python/lldbsuite/test/expression_command/warnings/TestExprWarnings.py
  lldb/packages/Python/lldbsuite/test/expression_command/warnings/main.cpp
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CommandObjectExpression.cpp
  lldb/source/Commands/CommandObjectExpression.h
  lldb/source/Commands/Options.td
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2335,7 +2335,7 @@
     llvm::StringRef expr, ExecutionContextScope *exe_scope,
     lldb::ValueObjectSP &result_valobj_sp,
     const EvaluateExpressionOptions &options, std::string *fixed_expression,
-    ValueObject *ctx_obj) {
+    ValueObject *ctx_obj, std::string *warnings) {
   result_valobj_sp.reset();
 
   ExpressionResults execution_results = eExpressionSetupError;
@@ -2385,7 +2385,7 @@
         UserExpression::Evaluate(exe_ctx, options, expr, prefix,
                                  result_valobj_sp, error, fixed_expression,
                                  nullptr, // Module
-                                 ctx_obj);
+                                 ctx_obj, warnings);
   }
 
   return execution_results;
Index: lldb/source/Expression/UserExpression.cpp
===================================================================
--- lldb/source/Expression/UserExpression.cpp
+++ lldb/source/Expression/UserExpression.cpp
@@ -143,7 +143,7 @@
     llvm::StringRef expr, llvm::StringRef prefix,
     lldb::ValueObjectSP &result_valobj_sp, Status &error,
     std::string *fixed_expression, lldb::ModuleSP *jit_module_sp_ptr,
-    ValueObject *ctx_obj) {
+    ValueObject *ctx_obj, std::string *warnings) {
   Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_EXPRESSIONS |
                                                   LIBLLDB_LOG_STEP));
 
@@ -273,6 +273,7 @@
       parse_success = fixed_expression_sp->Parse(
           fixed_diagnostic_manager, exe_ctx, execution_policy,
           keep_expression_in_memory, generate_debug_info);
+
       if (parse_success) {
         diagnostic_manager.Clear();
         user_expression_sp = fixed_expression_sp;
@@ -301,6 +302,9 @@
   }
 
   if (parse_success) {
+    if (warnings)
+      *warnings = diagnostic_manager.GetString();
+
     // If a pointer to a lldb::ModuleSP was passed in, return the JIT'ed module
     // if one was created
     if (jit_module_sp_ptr)
Index: lldb/source/Commands/Options.td
===================================================================
--- lldb/source/Commands/Options.td
+++ lldb/source/Commands/Options.td
@@ -353,6 +353,9 @@
     Desc<"When specified, debug the JIT code by setting a breakpoint on the "
     "first instruction and forcing breakpoints to not be ignored (-i0) and no "
     "unwinding to happen on error (-u0).">;
+  def expression_options_warning : Option<"warning", "w">, Groups<[1,2]>,
+    Desc<"Enables compiler warnings when parsing the expression, even when the "
+    "expression was successfully parsed.">;
   def expression_options_language : Option<"language", "l">, Groups<[1,2]>,
     Arg<"Language">, Desc<"Specifies the Language to use when parsing the "
     "expression.  If not set the target.language setting is used.">;
Index: lldb/source/Commands/CommandObjectExpression.h
===================================================================
--- lldb/source/Commands/CommandObjectExpression.h
+++ lldb/source/Commands/CommandObjectExpression.h
@@ -41,6 +41,7 @@
     bool show_types;
     bool show_summary;
     bool debug;
+    bool warning;
     uint32_t timeout;
     bool try_all_threads;
     lldb::LanguageType language;
Index: lldb/source/Commands/CommandObjectExpression.cpp
===================================================================
--- lldb/source/Commands/CommandObjectExpression.cpp
+++ lldb/source/Commands/CommandObjectExpression.cpp
@@ -143,6 +143,10 @@
           option_arg.str().c_str());
     break;
 
+  case 'w':
+    warning = true;
+    break;
+
   case 'g':
     debug = true;
     unwind_on_error = false;
@@ -188,6 +192,7 @@
   try_all_threads = true;
   timeout = 0;
   debug = false;
+  warning = false;
   language = eLanguageTypeUnknown;
   m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityCompact;
   auto_apply_fixits = eLazyBoolCalculate;
@@ -423,8 +428,12 @@
     else
       options.SetTimeout(llvm::None);
 
+    std::string warnings;
     ExpressionResults success = target->EvaluateExpression(
-        expr, frame, result_valobj_sp, options, &m_fixed_expression);
+        expr, frame, result_valobj_sp, options, &m_fixed_expression,
+        /*ctx*/ nullptr, &warnings);
+    if (error_stream && m_command_options.warning)
+      error_stream->Printf("%s", warnings.c_str());
 
     // We only tell you about the FixIt if we applied it.  The compiler errors
     // will suggest the FixIt if it parsed.
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2267,6 +2267,7 @@
             substrs=None,
             trace=False,
             error=False,
+            checkerror=False,
             matching=True,
             exe=True,
             inHistory=False):
@@ -2282,7 +2283,8 @@
         If the keyword argument error is set to True, it signifies that the API
         client is expecting the command to fail.  In this case, the error stream
         from running the command is retrieved and compared against the golden
-        input, instead.
+        input, instead. If the command is expected to succeed but the error
+        stream should be checked, checkerror should be set to True.
 
         If the keyword argument matching is set to False, it signifies that the API
         client is expecting the output of the command not to match the golden
@@ -2295,6 +2297,15 @@
         """
         trace = (True if traceAlways else trace)
 
+
+        # error=True and checkerror=True is redundant. Only use checkerror=True
+        # if you want the error output of a command that succeeds.
+        self.assertFalse(error and checkerror,
+                         "error=True and checkerror=True is redundant")
+
+        if error:
+          checkerror = True
+
         if exe:
             # First run the command.  If we are expecting error, set check=False.
             # Pass the assert message along since it provides more semantic
@@ -2308,7 +2319,7 @@
                 inHistory=inHistory)
 
             # Then compare the output against expected strings.
-            output = self.res.GetError() if error else self.res.GetOutput()
+            output = self.res.GetError() if checkerror else self.res.GetOutput()
 
             # If error is True, the API client expects the command to fail!
             if error:
Index: lldb/packages/Python/lldbsuite/test/expression_command/warnings/main.cpp
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/expression_command/warnings/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  int x = 22;
+  return 0; // Break here
+}
Index: lldb/packages/Python/lldbsuite/test/expression_command/warnings/TestExprWarnings.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/expression_command/warnings/TestExprWarnings.py
@@ -0,0 +1,43 @@
+"""
+Test the -w flag for the expression command that prints Clang warnings.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class ExprWarningsTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+        self.main_source = "main.cpp"
+        self.main_source_spec = lldb.SBFileSpec(self.main_source)
+        self.build()
+
+    def test_enabled_warnings(self):
+        """Test that we show expression warnings with the -w flag"""
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                          '// Break here', self.main_source_spec)
+        self.expect("expr -w -- int", checkerror=True, substrs=["declaration does not declare anything"])
+        self.expect("expr -w -- switch(true) { case 0: case 1: break; }", checkerror=True, substrs=["switch condition has boolean value"])
+
+    def test_disabled_warnings(self):
+        """Test that we don't show expression warnings by default"""
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                          '// Break here', self.main_source_spec)
+        self.expect("expr -- int", checkerror=True, matching=False, substrs=["declaration does not declare anything"])
+        self.expect("expr -- switch(true) { case 0: case 1: break; }", checkerror=True, matching=False, substrs=["switch condition has boolean value"])
+
+    def test_enabled_warnings_with_expr_without_warnings(self):
+        """Test that -w doesn't break anything with expressions that don't produce any warnings"""
+        self.build()
+
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                          '// Break here', self.main_source_spec)
+        self.expect("expr -w -- 1 + 1", substrs=[" = 2"])
+        self.expect("expr -w -- x = 44", substrs=[" = 44"])
Index: lldb/packages/Python/lldbsuite/test/expression_command/warnings/Makefile
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/expression_command/warnings/Makefile
@@ -0,0 +1,5 @@
+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1102,7 +1102,8 @@
       llvm::StringRef expression, ExecutionContextScope *exe_scope,
       lldb::ValueObjectSP &result_valobj_sp,
       const EvaluateExpressionOptions &options = EvaluateExpressionOptions(),
-      std::string *fixed_expression = nullptr, ValueObject *ctx_obj = nullptr);
+      std::string *fixed_expression = nullptr, ValueObject *ctx_obj = nullptr,
+      std::string *warnings = nullptr);
 
   lldb::ExpressionVariableSP GetPersistentVariable(ConstString name);
 
Index: lldb/include/lldb/Expression/UserExpression.h
===================================================================
--- lldb/include/lldb/Expression/UserExpression.h
+++ lldb/include/lldb/Expression/UserExpression.h
@@ -255,6 +255,10 @@
   ///     Currently there is a limitation: the context object must be located
   ///     in the debuggee process' memory (and have the load address).
   ///
+  /// \param[out] warnings
+  ///     If non-nullptr, the warnings generated by the expression is copied
+  ///     into the provided string.
+  ///
   /// \result
   ///      A Process::ExpressionResults value.  eExpressionCompleted for
   ///      success.
@@ -264,7 +268,7 @@
            lldb::ValueObjectSP &result_valobj_sp, Status &error,
            std::string *fixed_expression = nullptr,
            lldb::ModuleSP *jit_module_sp_ptr = nullptr,
-           ValueObject *ctx_obj = nullptr);
+           ValueObject *ctx_obj = nullptr, std::string *warnings = nullptr);
 
   static const Status::ValueType kNoResult =
       0x1001; ///< ValueObject::GetError() returns this if there is no result
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to