This is an archive of the discontinued LLVM Phabricator instance.

Add code snippet line numbers to TestExprDiagnostics output
ClosedPublic

Authored by tbaeder on May 17 2023, 5:22 AM.

Details

Reviewers
JDevlieghere
Group Reviewers
Restricted Project
Summary

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.

Diff Detail

Event Timeline

tbaeder created this revision.May 17 2023, 5:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 5:22 AM
tbaeder requested review of this revision.May 17 2023, 5:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 17 2023, 5:22 AM
teemperor added a subscriber: teemperor.EditedMay 17 2023, 5:46 AM

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

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.

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

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.

tbaeder updated this revision to Diff 523045.May 17 2023, 7:19 AM

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

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.

tbaeder updated this revision to Diff 526557.May 30 2023, 2:53 AM
This revision is now accepted and ready to land.May 30 2023, 10:32 AM
tbaeder closed this revision.May 30 2023, 10:58 PM

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