Collect CPP error logs into diagnostics. #296
Conversation
jinwoo
force-pushed
the
cpp
branch
3 times, most recently
from
e8810d3
to
277237d
Compare
|
The source location of the CPP error from CI is different from what I see locally. It is (2, 1) locally but (2, -1) in CI. I'll change the column number to |
| $ liftIO | ||
| $ (Right <$> (runCpp dflags {log_action = logAction cppLogs} filename | ||
| $ if isOnDisk then Nothing else Just contents)) | ||
| `catch` (\(e :: SomeException) -> do |
There was a problem hiding this comment.
Please don’t catch SomeException. We use async exceptions very heavily to kill the currently running computation and code that catches SomeException will also catch ThreadKilled.
We already depend on safe-exceptions so catchIO might be a reasonable option or if this really does throw exceptions other than IOException, catch from Control.Exception.Safe will not catch async exceptions.
There was a problem hiding this comment.
Good point. I traced the code and figured that the exceptions thrown from runCpp are GhcException. Updated. And now using catch and throw from safe-exception.
test/exe/Main.hs
Outdated
| T.unlines | ||
| [ " error: unterminated #ifdef", | ||
| " #ifdef FOO", | ||
| " ^" |
There was a problem hiding this comment.
My CPP doesn’t seem to have the carret in the output here so it would probably be good to make this test a bit more fuzzy and only assert on error: unterminated #ifdef which is hopefully present in the output of most if not all CPP implementations.
There was a problem hiding this comment.
Reduced to error: unterminated because the error message on my mac is error: unterminated conditional directive.
test/exe/Main.hs
Outdated
| expectDiagnostics | ||
| [ ( "Testing.hs", | ||
| [ ( DsError, | ||
| (2, -1), |
There was a problem hiding this comment.
I get the same line numbers as CI but a different failure as mentioned below. My guess is that this is because we’re using different CPP implementations (I’m using whatever the default is on linux when invoked via stack, I guess you might be on Windows or MacOS). I would like to avoid this test being flaky locally so maybe we can just test against both (2,-1) and (2,0) (by catching HUnitFailure on the first try) and succeed if either of them succeeds?
There was a problem hiding this comment.
Yeah, I'm doing this on my mac at the moment. What about this?
* Collect CPP error logs into diagnostics. Fixes https://github.com/digital-asset/ghcide/issues/87
* Collect CPP error logs into diagnostics. Fixes https://github.com/digital-asset/ghcide/issues/87
* Collect CPP error logs into diagnostics. Fixes https://github.com/digital-asset/ghcide/issues/87
* Collect CPP error logs into diagnostics. Fixes https://github.com/digital-asset/ghcide/issues/87
Fixes https://github.com/digital-asset/ghcide/issues/87