Index: clang/include/clang/Basic/DiagnosticOptions.def =================================================================== --- clang/include/clang/Basic/DiagnosticOptions.def +++ clang/include/clang/Basic/DiagnosticOptions.def @@ -49,6 +49,7 @@ DIAGOPT(PedanticErrors, 1, 0) /// -pedantic-errors DIAGOPT(ShowColumn, 1, 1) /// Show column number on diagnostics. DIAGOPT(ShowLocation, 1, 1) /// Show source location information. +DIAGOPT(ShowLevel, 1, 1) /// Show diagnostic level. DIAGOPT(AbsolutePath, 1, 0) /// Use absolute paths. DIAGOPT(ShowCarets, 1, 1) /// Show carets in diagnostics. DIAGOPT(ShowFixits, 1, 1) /// Show fixit information. Index: clang/lib/Frontend/TextDiagnostic.cpp =================================================================== --- clang/lib/Frontend/TextDiagnostic.cpp +++ clang/lib/Frontend/TextDiagnostic.cpp @@ -676,8 +676,9 @@ if (DiagOpts->ShowColors) OS.resetColor(); - printDiagnosticLevel(OS, Level, DiagOpts->ShowColors, - DiagOpts->CLFallbackMode); + if (DiagOpts->ShowLevel) + printDiagnosticLevel(OS, Level, DiagOpts->ShowColors, + DiagOpts->CLFallbackMode); printDiagnosticMessage(OS, /*IsSupplemental*/ Level == DiagnosticsEngine::Note, Message, OS.tell() - StartOfLocationInfo, Index: lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/Makefile =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/Makefile @@ -0,0 +1,5 @@ +LEVEL = ../../make + +CXX_SOURCES := main.cpp + +include $(LEVEL)/Makefile.rules Index: lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/TestExprDiagnostics.py =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/TestExprDiagnostics.py @@ -0,0 +1,81 @@ +""" +Test the diagnostics emitted by our embeded Clang instance that parses expressions. +""" + +import lldb +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class ExprDiagnosticsTestCase(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) + + def test_source_and_caret_printing(self): + """Test that the source and caret positions LLDB prints are correct""" + self.build() + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + '// Break here', self.main_source_spec) + frame = thread.GetFrameAtIndex(0) + + # Test that source/caret are at the right position. + value = frame.EvaluateExpression("unknown_identifier") + self.assertFalse(value.GetError().Success()) + # We should get a nice diagnostic with a caret pointing at the start of + # the identifier. + self.assertIn("\nunknown_identifier\n^\n", value.GetError().GetCString()) + self.assertIn(":1:1", value.GetError().GetCString()) + + # Same as above but with the identifier in the middle. + value = frame.EvaluateExpression("1 + unknown_identifier ") + self.assertFalse(value.GetError().Success()) + self.assertIn("\n1 + unknown_identifier", value.GetError().GetCString()) + self.assertIn("\n ^\n", value.GetError().GetCString()) + + # Multiline expressions. + value = frame.EvaluateExpression("int a = 0;\nfoobar +=1;\na") + self.assertFalse(value.GetError().Success()) + # We should still get the right line information and caret position. + self.assertIn("\nfoobar +=1;\n^\n", value.GetError().GetCString()) + # It's the second line of the user expression. + self.assertIn(":2:1", value.GetError().GetCString()) + + # Top-level expressions. + top_level_opts = lldb.SBExpressionOptions(); + top_level_opts.SetTopLevel(True) + + value = frame.EvaluateExpression("void foo(unknown_type x) {}", top_level_opts) + self.assertFalse(value.GetError().Success()) + self.assertIn("\nvoid foo(unknown_type x) {}\n ^\n", value.GetError().GetCString()) + # Top-level expressions might use a different wrapper code, but the file name should still + # be the same. + self.assertIn(":1:10", value.GetError().GetCString()) + + # Multiline top-level expressions. + value = frame.EvaluateExpression("void x() {}\nvoid foo(unknown_type x) {}", top_level_opts) + self.assertFalse(value.GetError().Success()) + self.assertIn("\nvoid foo(unknown_type x) {}\n ^\n", value.GetError().GetCString()) + self.assertIn(":2:10", value.GetError().GetCString()) + + # Test that we render Clang's 'notes' correctly. + value = frame.EvaluateExpression("struct SFoo{}; struct SFoo { int x; };", top_level_opts) + self.assertFalse(value.GetError().Success()) + self.assertIn(":1:8: previous definition is here\nstruct SFoo{}; struct SFoo { int x; };\n ^\n", value.GetError().GetCString()) + + # Declarations from the debug information currently have no debug information. It's not clear what + # we should do in this case, but we should at least not print anything that's wrong. + # In the future our declarations should have valid source locations. + value = frame.EvaluateExpression("struct FooBar { double x };", top_level_opts) + self.assertFalse(value.GetError().Success()) + self.assertEqual("error: :1:8: redefinition of 'FooBar'\nstruct FooBar { double x };\n ^\n", value.GetError().GetCString()) + + value = frame.EvaluateExpression("foo(1, 2)") + self.assertFalse(value.GetError().Success()) + self.assertEqual("error: :1:1: no matching function for call to 'foo'\nfoo(1, 2)\n^~~\nnote: candidate function not viable: requires single argument 'x', but 2 arguments were provided\n\n", value.GetError().GetCString()) Index: lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/main.cpp =================================================================== --- /dev/null +++ lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/main.cpp @@ -0,0 +1,11 @@ +void foo(int x) {} + +struct FooBar { + int i; +}; + +int main() { + FooBar f; + foo(1); + return 0; // Break here +} Index: lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods.py =================================================================== --- lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods.py +++ lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods.py @@ -193,7 +193,7 @@ "expression self->non_existent_member", COMMAND_FAILED_AS_EXPECTED, error=True, - startstr="error: 'MyString' does not have a member named 'non_existent_member'") + substrs=["error:", "'MyString' does not have a member named 'non_existent_member'"]) # Use expression parser. self.runCmd("expression self->str") Index: lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h +++ lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h @@ -29,7 +29,7 @@ return diag->getKind() == eDiagnosticOriginClang; } - ClangDiagnostic(const char *message, DiagnosticSeverity severity, + ClangDiagnostic(llvm::StringRef message, DiagnosticSeverity severity, uint32_t compiler_id) : Diagnostic(message, severity, eDiagnosticOriginClang, compiler_id) {} Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -137,12 +137,14 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { public: - ClangDiagnosticManagerAdapter() - : m_passthrough(new clang::TextDiagnosticBuffer) {} - - ClangDiagnosticManagerAdapter( - const std::shared_ptr &passthrough) - : m_passthrough(passthrough) {} + ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) { + DiagnosticOptions *m_options = new DiagnosticOptions(opts); + m_options->ShowPresumedLoc = true; + m_options->ShowLevel = false; + m_os.reset(new llvm::raw_string_ostream(m_output)); + m_passthrough.reset( + new clang::TextDiagnosticPrinter(*m_os, m_options, false)); + } void ResetManager(DiagnosticManager *manager = nullptr) { m_manager = manager; @@ -150,12 +152,11 @@ void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) override { - if (m_manager) { - llvm::SmallVector diag_str; - Info.FormatDiagnostic(diag_str); - diag_str.push_back('\0'); - const char *data = diag_str.data(); + m_output.clear(); + m_passthrough->HandleDiagnostic(DiagLevel, Info); + m_os->flush(); + if (m_manager) { lldb_private::DiagnosticSeverity severity; bool make_new_diagnostic = true; @@ -172,12 +173,16 @@ severity = eDiagnosticSeverityRemark; break; case DiagnosticsEngine::Level::Note: - m_manager->AppendMessageToDiagnostic(data); + m_manager->AppendMessageToDiagnostic(m_output); make_new_diagnostic = false; } if (make_new_diagnostic) { + // ClangDiagnostic messages are expected to have no whitespace/newlines + // around them. + std::string stripped_output = llvm::StringRef(m_output).trim(); + ClangDiagnostic *new_diagnostic = - new ClangDiagnostic(data, severity, Info.getID()); + new ClangDiagnostic(stripped_output, severity, Info.getID()); m_manager->AddDiagnostic(new_diagnostic); // Don't store away warning fixits, since the compiler doesn't have @@ -194,23 +199,15 @@ } } } - - m_passthrough->HandleDiagnostic(DiagLevel, Info); - } - - void FlushDiagnostics(DiagnosticsEngine &Diags) { - m_passthrough->FlushDiagnostics(Diags); - } - - DiagnosticConsumer *clone(DiagnosticsEngine &Diags) const { - return new ClangDiagnosticManagerAdapter(m_passthrough); } - clang::TextDiagnosticBuffer *GetPassthrough() { return m_passthrough.get(); } + clang::TextDiagnosticPrinter *GetPassthrough() { return m_passthrough.get(); } private: DiagnosticManager *m_manager = nullptr; - std::shared_ptr m_passthrough; + std::shared_ptr m_passthrough; + std::string m_output; + std::shared_ptr m_os; }; static void @@ -558,7 +555,9 @@ // 6. Set up the diagnostic buffer for reporting errors - m_compiler->getDiagnostics().setClient(new ClangDiagnosticManagerAdapter); + auto diag_mgr = new ClangDiagnosticManagerAdapter( + m_compiler->getDiagnostics().getDiagnosticOptions()); + m_compiler->getDiagnostics().setClient(diag_mgr); // 7. Set up the source management objects inside the compiler m_compiler->createFileManager(); @@ -870,8 +869,7 @@ ClangDiagnosticManagerAdapter *adapter = static_cast( m_compiler->getDiagnostics().getClient()); - clang::TextDiagnosticBuffer *diag_buf = adapter->GetPassthrough(); - diag_buf->FlushDiagnostics(m_compiler->getDiagnostics()); + auto diag_buf = adapter->GetPassthrough(); adapter->ResetManager(&diagnostic_manager); @@ -922,7 +920,7 @@ if (!created_main_file) { std::unique_ptr memory_buffer = - MemoryBuffer::getMemBufferCopy(expr_text, ""); + MemoryBuffer::getMemBufferCopy(expr_text, ""); source_mgr.setMainFileID(source_mgr.createFileID(std::move(memory_buffer))); } Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp @@ -29,7 +29,9 @@ using namespace lldb_private; -const char *ClangExpressionSourceCode::g_expression_prefix = R"( +const char *ClangExpressionSourceCode::g_expression_prefix = + R"( +#line 1 "" #ifndef offsetof #define offsetof(t, d) __builtin_offsetof(t, d) #endif @@ -67,8 +69,8 @@ } )"; -static const char *c_start_marker = " /*LLDB_BODY_START*/\n "; -static const char *c_end_marker = ";\n /*LLDB_BODY_END*/\n"; +static const char *c_start_marker = "#line 1 \"\"\n"; +static const char *c_end_marker = "\n; /*LLDB_BODY_END*/\n"; namespace {