[lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds.

[teemperor]

Summary

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).

[labath]

You could avoid the business with adding a flag to self.expect (which has enough flags already) by just making this a lit test. :)

[JDevlieghere]

lldb/include/lldb/Target/Target.h

1106

Is there anyway we could store this and things like the fixed expression in the ExpressionResults and then specify in the EvaluateExpressionOptions whether we want this to be set or not?

[shafik]

lldb/source/Expression/UserExpression.cpp

276

Is this just white space?

lldb/include/lldb/Expression/UserExpression.h

public:
   ///     this object. It means that the context object's address will be
   ///     treated as `this` for the expression (the expression will be
   ///     evaluated as if it was inside of a method of the context object's
   ///     class, and its `this` parameter were pointing to the context object).
   ///     The parameter makes sense for class and union types only.
   ///     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.
   static lldb::ExpressionResults
   Evaluate(ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
            llvm::StringRef expr_cstr, llvm::StringRef expr_prefix,
            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
               /// from the expression.
 
   const char *GetFixedText() {
     if (m_fixed_text.empty())
       return nullptr;

lldb/include/lldb/Target/Target.h

public:
   // and the const expression results are available after a process is gone, we
   // provide a way for expressions to be evaluated from the Target itself. If
   // an expression is going to be run, then it should have a frame filled in in
   // the execution context.
   lldb::ExpressionResults EvaluateExpression(
       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);
 
   lldb::addr_t GetPersistentSymbol(ConstString name);
 
   /// This method will return the address of the starting function for
   /// this binary, e.g. main() or its equivalent.  This can be used as
   /// an address of a function that is not called once a binary has

lldb/packages/Python/lldbsuite/test/expression_command/warnings/Makefile

+LEVEL = ../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

lldb/packages/Python/lldbsuite/test/expression_command/warnings/TestExprWarnings.py

+"""
+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"])

lldb/packages/Python/lldbsuite/test/expression_command/warnings/main.cpp

+int main() {
+  int x = 22;
+  return 0; // Break here
+}

lldb/packages/Python/lldbsuite/test/lldbtest.py

def expect(
             str,
             msg=None,
             patterns=None,
             startstr=None,
             endstr=None,
             substrs=None,
             trace=False,
             error=False,
+            checkerror=False,
             matching=True,
             exe=True,
             inHistory=False):
         """
         Similar to runCmd; with additional expect style output matching ability.
 
         Ask the command interpreter to handle the command and then check its
         return status.  The 'msg' parameter specifies an informational assert
         message.  We expect the output from running the command to start with
         'startstr', matches the substrings contained in 'substrs', and regexp
         matches the patterns contained in 'patterns'.
 
         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
         input.
 
         Finally, the required argument 'str' represents the lldb command to be
         sent to the command interpreter.  In case the keyword argument 'exe' is
         set to False, the 'str' is treated as a string to be matched/not-matched
         against the golden input.
         """
         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
             # info.
             self.runCmd(
                 str,
                 msg=msg,
                 trace=(
                     True if trace else False),
                 check=not error,
                 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:
                 self.assertFalse(self.res.Succeeded(),
                                  "Command '" + str + "' is expected to fail!")
         else:
             # No execution required, just compare str against the golden input.
             if isinstance(str, lldb.SBCommandReturnObject):

lldb/source/Commands/CommandObjectExpression.h

public:
 
     bool top_level;
     bool unwind_on_error;
     bool ignore_breakpoints;
     bool allow_jit;
     bool show_types;
     bool show_summary;
     bool debug;
+    bool warning;
     uint32_t timeout;
     bool try_all_threads;
     lldb::LanguageType language;
     LanguageRuntimeDescriptionDisplayVerbosity m_verbosity;
     LazyBool auto_apply_fixits;
   };
 
   CommandObjectExpression(CommandInterpreter &interpreter);

lldb/source/Commands/CommandObjectExpression.cpp

m_verbosity = (LanguageRuntimeDescriptionDisplayVerbosity)
         OptionArgParser::ToOptionEnum(
             option_arg, GetDefinitions()[option_idx].enum_values, 0, error);
     if (!error.Success())
       error.SetErrorStringWithFormat(
           "unrecognized value for description-verbosity '%s'",
           option_arg.str().c_str());
     break;
 
+  case 'w':
+    warning = true;
+    break;
+
   case 'g':
     debug = true;
     unwind_on_error = false;
     ignore_breakpoints = false;
     break;
 
   case 'p':
     top_level = true;
if (process_sp) {
     ignore_breakpoints = true;
     unwind_on_error = true;
   }
 
   show_summary = true;
   try_all_threads = true;
   timeout = 0;
   debug = false;
+  warning = false;
   language = eLanguageTypeUnknown;
   m_verbosity = eLanguageRuntimeDescriptionDisplayVerbosityCompact;
   auto_apply_fixits = eLazyBoolCalculate;
   top_level = false;
   allow_jit = true;
 }
 
 llvm::ArrayRef<OptionDefinition>
if (!m_command_options.ignore_breakpoints ||
         !m_command_options.unwind_on_error)
       options.SetGenerateDebugInfo(true);
 
     if (m_command_options.timeout > 0)
       options.SetTimeout(std::chrono::microseconds(m_command_options.timeout));
     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.
     if (error_stream && !m_fixed_expression.empty() &&
         target->GetEnableNotifyAboutFixIts()) {
       if (success == eExpressionCompleted)
         error_stream->Printf(
             "  Fix-it applied, fixed expression was: \n    %s\n",

lldb/source/Commands/Options.td

def expression_options_unwind_on_error : Option<"unwind-on-error", "u">,
     Groups<[1,2]>, Arg<"Boolean">,
     Desc<"Clean up program state if the expression causes a crash, or raises a "
     "signal. Note, unlike gdb hitting a breakpoint is controlled by another "
     "option (-i).">;
   def expression_options_debug : Option<"debug", "g">, Groups<[1,2]>,
     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.">;
   def expression_options_apply_fixits : Option<"apply-fixits", "X">,
     Groups<[1,2]>, Arg<"Language">, Desc<"If true, simple fix-it hints will be "
     "automatically applied to the expression.">;
   def expression_options_description_verbosity :
     Option<"description-verbosity", "v">, Group<1>,

lldb/source/Expression/UserExpression.cpp

lldb::addr_t UserExpression::GetObjectPointer(lldb::StackFrameSP frame_sp,
   return ret;
 }
 
 lldb::ExpressionResults UserExpression::Evaluate(
     ExecutionContext &exe_ctx, const EvaluateExpressionOptions &options,
     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));
 
   if (ctx_obj) {
     static unsigned const ctx_type_mask =
         lldb::TypeFlags::eTypeIsClass | lldb::TypeFlags::eTypeIsStructUnion;
     if (!(ctx_obj->GetTypeInfo() & ctx_type_mask)) {
       LLDB_LOG(log, "== [UserExpression::Evaluate] Passed a context object of "
if (fixed_expression && !fixed_expression->empty() &&
           target->GetUserExpressionForLanguage(fixed_expression->c_str(),
                                                full_prefix, language,
                                                desired_type, options, ctx_obj,
                                                error));
       DiagnosticManager fixed_diagnostic_manager;
       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;
       } else {
         // If the fixed expression failed to parse, don't tell the user about,
         // that won't help.
         fixed_expression->clear();
       }
if (!parse_success) {
         else
           error.SetExpressionError(execution_results,
                                    diagnostic_manager.GetString().c_str());
       }
     }
   }
 
   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)
       *jit_module_sp_ptr = user_expression_sp->GetJITModule();
 
     lldb::ExpressionVariableSP expr_result;
 
     if (execution_policy == eExecutionPolicyNever &&

lldb/source/Target/Target.cpp

if (target == nullptr && exe_ctx_ptr)
     target = exe_ctx_ptr->GetTargetPtr();
   return target;
 }
 
 ExpressionResults Target::EvaluateExpression(
     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;
 
   if (expr.empty())
     return execution_results;
 
   // We shouldn't run stop hooks in expressions.
if (persistent_var_sp) {
     execution_results = eExpressionCompleted;
   } else {
     llvm::StringRef prefix = GetExpressionPrefixContents();
     Status error;
     execution_results =
         UserExpression::Evaluate(exe_ctx, options, expr, prefix,
                                  result_valobj_sp, error, fixed_expression,
                                  nullptr, // Module
-                                 ctx_obj);
+                                 ctx_obj, warnings);
   }
 
   return execution_results;
 }
 
 lldb::ExpressionVariableSP Target::GetPersistentVariable(ConstString name) {
   lldb::ExpressionVariableSP variable_sp;
   m_scratch_type_system_map.ForEach(