teemperor created this revision.
teemperor added a reviewer: LLDB.
Herald added subscribers: lldb-commits, kadircet, arphaman, aprantl.
Herald added a project: LLDB.
Currently our expression evaluators only prints very basic errors that are not
very useful when writing complex expressions.
For example, in the expression below the user made a type error, but it's not
clear from the diagnostic what went wrong:
(lldb) expr printf("Modulos are:", foobar%mo1, foobar%mo2, foobar%mo3)
error: invalid operands to binary expression ('int' and 'double')
This patch enables full Clang diagnostics in our expression evaluator. After
this patch the diagnostics for the expression look like this:
(lldb) expr printf("Modulos are:", foobar%mo1, foobar%mo2, foobar%mo3)
error: <user expression>:1:54: invalid operands to binary expression ('int'
and 'float')
printf("Modulos are:", foobar%mo1, foobar%mo2, foobar%mo3)
~~~~~~^~~~
To make this possible, we now emulate a user expression file within our
diagnostics. This prevents that the user is exposed to
our internal wrapper code we inject.
Note that the diagnostics that refer to declarations from the debug information
(e.g. 'note' diagnostics pointing to a called function)
will not be improved by this as they don't have any source locations associated
with them, so caret or line printing isn't possible.
We instead just suppress these diagnostics as we already do with warnings as
they would otherwise just be a context message
without any context (and the original diagnostic in the user expression should
be enough to explain the issue).
Fixes rdar://24306342
Repository:
rLLDB LLDB
https://reviews.llvm.org/D65646
Files:
clang/include/clang/Basic/DiagnosticOptions.def
clang/lib/Frontend/TextDiagnostic.cpp
lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/Makefile
lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/TestExprDiagnostics.py
lldb/packages/Python/lldbsuite/test/expression_command/diagnostics/main.cpp
lldb/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods.py
lldb/source/Plugins/ExpressionParser/Clang/ClangDiagnostic.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
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 "<lldb wrapper code>"
#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 \"<user expression>\"\n";
+static const char *c_end_marker = "\n; /*LLDB_BODY_END*/\n";
namespace {
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<clang::TextDiagnosticBuffer> &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<char, 32> 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<clang::TextDiagnosticBuffer> m_passthrough;
+ std::shared_ptr<clang::TextDiagnosticPrinter> m_passthrough;
+ std::string m_output;
+ std::shared_ptr<llvm::raw_string_ostream> 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<ClangDiagnosticManagerAdapter *>(
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<MemoryBuffer> memory_buffer =
- MemoryBuffer::getMemBufferCopy(expr_text, "<lldb-expr>");
+ MemoryBuffer::getMemBufferCopy(expr_text, "<user expression>");
source_mgr.setMainFileID(source_mgr.createFileID(std::move(memory_buffer)));
}
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/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/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/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("<user expression>: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("<user expression>: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("<user expression>: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("<user expression>: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("<user expression>: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: <user expression>: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: <user expression>: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/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: 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: 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.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
