[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder closed this revision.
tbaeder added a comment.

I merged the changes into the original line number commit and pushed them as 
https://github.com/llvm/llvm-project/commit/f63155aaa6467bd2610820dfd1996af3bb6029a7


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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 526557.

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

https://reviews.llvm.org/D150772

Files:
  lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

Index: lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
===
--- lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -29,20 +29,38 @@
 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 | unknown_identifier
+  | ^
+""",
+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  ")
+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())
+self.assertIn(
+"""
+1 | 1 + unknown_identifier
+  | ^
+""",
+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())
+self.assertIn(
+"""
+2 | foobar +=1;
+  | ^
+""",
+value.GetError().GetCString(),
+)
+
 # It's the second line of the user expression.
 self.assertIn(":2:1", value.GetError().GetCString())
 
@@ -52,7 +70,14 @@
 
 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())
+self.assertIn(
+"""
+1 | void foo(unknown_type x) {}
+  |  ^
+""",
+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())
@@ -60,31 +85,79 @@
 # Multiline top-level expressions.
 value = frame.EvaluateExpression("void x() {}\nvoid foo;", top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
+self.assertIn(
+"""
+2 | void foo;
+  |  ^
+""",
+value.GetError().GetCString(),
+)
+
 self.assertIn(":2:6", 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())
+self.assertIn(
+":1:8: previous definition is here\n",
+value.GetError().GetCString(),
+)
+self.assertIn(
+"""
+1 | struct SFoo{}; struct SFoo { int x; };
+  |^
+""",
+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.assertIn("error: :1:8: redefinition of 'FooBar'\nstruct FooBar { double x };\n   ^\n", value.GetError().GetCString())
+self.assertIn(
+"error: :1:8: redefinition of 'FooBar'\n",
+value.GetError().GetCString(),
+)
+self.assertIn(
+"""
+1 | struct FooBar { double x };
+  |^
+""",
+value.GetError().GetCString(),
+)
 
 value = frame.EvaluateExpression("foo(1, 2)")
 self.assertFalse(value.GetError().Success())
-self.assertIn("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 argumen

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

I think this test could benefit from switching to string literals (`'''`) and 
doing a single `self.assertIn` rather than scanning the whole output for every 
line. Also please use `black` (version 23) to format the test.


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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 523045.
tbaeder added a comment.

Updated the test and split up the expected output into multiple lines so it's 
easier to read.


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

https://reviews.llvm.org/D150772

Files:
  lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py


Index: lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
===
--- lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -1,5 +1,5 @@
 """
-Test the diagnostics emitted by our embeded Clang instance that parses 
expressions.
+Test the diagnostics emitted by our embedded Clang instance that parses 
expressions.
 """
 
 import lldb
@@ -29,20 +29,21 @@
 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("\n1 | unknown_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())
+self.assertIn("\n1 | 1 + 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())
+self.assertIn("\n2 | foobar +=1;\n", value.GetError().GetCString())
+self.assertIn("\n  | ^\n", value.GetError().GetCString())
 # It's the second line of the user expression.
 self.assertIn(":2:1", value.GetError().GetCString())
 
@@ -52,7 +53,8 @@
 
 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())
+self.assertIn("\n1 | void foo(unknown_type x) {}\n", 
value.GetError().GetCString())
+self.assertIn("\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())
@@ -60,31 +62,44 @@
 # Multiline top-level expressions.
 value = frame.EvaluateExpression("void x() {}\nvoid foo;", 
top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
+self.assertIn("\n2 | void foo;\n", value.GetError().GetCString())
+self.assertIn("\n  |  ^", value.GetError().GetCString())
 self.assertIn(":2:6", 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())
+self.assertIn(":1:8: previous definition is 
here\n", value.GetError().GetCString())
+self.assertIn("\n1 | struct SFoo{}; struct SFoo { int x; };\n", 
value.GetError().GetCString())
+self.assertIn("\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.assertIn("error: :1:8: redefinition of 
'FooBar'\nstruct FooBar { double x };\n   ^\n", 
value.GetError().GetCString())
+self.assertIn("error: :1:8: redefinition of 
'FooBar'\n", value.GetError().GetCString())
+self.assertIn("\n1 | struct FooBar { double x };\n", 
value.GetError().GetCString())
+self.assertIn("\n  |^\n", value.GetError().GetCString())
 
 val

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder added a comment.

In D150772#4349595 , @Michael137 
wrote:

> In D150772#4349592 , @tbaeder wrote:
>
>> Can you tell me what I have to do so `ninja check-lldb-api` actually runs 
>> the test? All 1114 tests are currently marked "unsupported" for me.
>
> Did you configure LLDB with `-DLLDB_ENABLE_PYTHON`? We should probably add 
> that info to 
> https://lldb.llvm.org/resources/test.html#running-the-full-test-suite

Ah yes, that seems to be the problem. Looks like that won't work when using 
sanitizers though, so I'll have to do another build and it will take a little 
longer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D150772#4349592 , @tbaeder wrote:

> Can you tell me what I have to do so `ninja check-lldb-api` actually runs the 
> test? All 1114 tests are currently marked "unsupported" for me.

Did you configure LLDB with `-DLLDB_ENABLE_PYTHON`? We should probably add that 
info to https://lldb.llvm.org/resources/test.html#running-the-full-test-suite


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder added a comment.

Can you tell me what I have to do so `ninja check-lldb-api` actually runs the 
test? All 1114 tests are currently marked "unsupported" for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

I think, given the support for multi-line expressions, updating the test case 
is IMHO the right call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150772

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D150772: Add code snippet line numbers to TestExprDiagnostics output

[Timm Bäder via Phabricator via lldb-commits]

tbaeder created this revision.
tbaeder added a reviewer: LLDB.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Clang prints a margin with line numbers to the left of code snippets now, 
introduced in https://reviews.llvm.org/D147875.

This is a patch to fix a lldb test case that was broken due to the above change 
in clang. We either need to tell clang //not// to output line numbers in 
diagnostic code snippets, or the test case needs to be adapted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150772

Files:
  lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py


Index: lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
===
--- lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
+++ lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
@@ -29,20 +29,20 @@
 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("\n1 | unknown_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())
+self.assertIn("\n1 | 1 + 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())
+self.assertIn("\n1 | foobar +=1;\n  | ^\n", 
value.GetError().GetCString())
 # It's the second line of the user expression.
 self.assertIn(":2:1", value.GetError().GetCString())
 
@@ -52,7 +52,7 @@
 
 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())
+self.assertIn("\n1 | void 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())
@@ -60,31 +60,31 @@
 # Multiline top-level expressions.
 value = frame.EvaluateExpression("void x() {}\nvoid foo;", 
top_level_opts)
 self.assertFalse(value.GetError().Success())
-self.assertIn("\nvoid foo;\n ^", value.GetError().GetCString())
+self.assertIn("\n2 | void foo;\n  |  ^", 
value.GetError().GetCString())
 self.assertIn(":2:6", 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())
+self.assertIn(":1:8: previous definition is here\n  
  1 | struct 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.assertIn("error: :1:8: redefinition of 
'FooBar'\nstruct FooBar { double x };\n   ^\n", 
value.GetError().GetCString())
+self.assertIn("error: :1:8: redefinition of 
'FooBar'\1 | nstruct FooBar { double x };\n  |^\n", 
value.GetError().GetCString())
 
 value = frame.EvaluateExpression("foo(1, 2)")
 self.assertFalse(value.GetError().Success())
-self.assertIn("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 we