Diagnostic handling is unstable

[compnerd]

Describe the bug The reported diagnostics can be mishandled, reporting diagnostics poorly or incorrectly. Observed behaviour includes:

• truncation of error message:

value of type 'Slice<UnsafeMutableBufferPointer<...>>' (aka 'Slice<UnsafeMutableBufferPointer<...>>') has no swiftc

• incorrect joining of lines/indexing (reporting the file as):

\^~~~~~~S:\SourceCache\...

• reporting of types as sources

Swift.numericCast \
  in call to function 'numericCast' swiftc [Ln 1, Col 24]

To Reproduce Steps to reproduce the behavior:

1. Build a large project with a larger set of errors
2. Compare problem window
3. ...
4. Profit?

I haven't been able to reproduce it very well as any change will alter the behaviour). I suspect that it requires overly long message with a large stream.

Expected behavior Correct reporting of diagnostics

Environment

• OS: Windows 10
• Swift version: Swift version 5.8-dev (LLVM 67ca5c03f10a5f9, Swift 54102ef207bddf4)
• Visual Studio Code version: 1.67.2
• vscode-swift version: 0.5.3

Additional context This is likely due to improper handling of output which can be interleaved when it exceeds the buffer size on the reader or pipe. (This was a problem within SPM even, where we had incorrect assumptions about the reporting of diagnostics from llbuild). An option may be to use serialized diagnostics which would give us better behaviour for buffering and allow us to control the rendering.

[adam-fowler]

Can you include the original error text from the swift compiler output with the issues ie what was output to the terminal. It'll give me an idea of the format of the text.

The parsing of error messages uses the following regex on each line of output. If you can read regex

^(.*):(\\d+):(\\d+):\\s+(warning|error|note):\\s+(.*)$

[compnerd]

I would, but I need to reproduce it first (hence the silly reproduction steps). If I am able to notice it again, I shall share that.

[compnerd]

I don't have a reproduction yet. I did hit it again but couldn't get the log as the content scrolled out of the backlog. However, the process made me notice something - the region being improperly sized. It appears that they log was force wrapped to 80-columns. I think that this possibly only occurs on the first build run. Subsequent runs seems to render with the proper terminal width.

[adam-fowler]

I have just fixed an issue where the buffer javascript was using was limited to 1MB. If your test output was greater than 1MB then it would lose test results. Fix was just up the buffer size to 16MB. This will be in v0.6.0 which is about to be released

[adam-fowler]

Also if you do overflow the buffer this will be reported now, instead of just ignored

[adam-fowler]

@compnerd Have you seen these issues since?

[compnerd]

I've not, but I've also not been using it as heavily nor have I tried it on a large enough failing build (I had enough patches on NIO where I was mostly able to build most of it).

[adam-fowler]

@compnerd any update on this. I haven't seen this at all

[compnerd]

No, I had only seen this with NIO and I shelved it again due to the difficulty of getting the changes merged.

[adam-fowler]

I just tried to compile NIO, but it only returned one error in NIOConcurrencyHelpers. It didn't even bother compiling anything else, so wasn't a great test. I'm going to close this until we get another repro