cxx-qt-build: do not panic for cxx_gen failures instead use Result #461
Conversation
|
FWIW to test this i added the following method to the minimal example without the extern block for the QUrl so it is missing. #[qinvokable]
pub fn test(&self, _url: &QUrl) {
}Then this is the full error, you can see how the first error is the correct one, the others are a side effect of the CXX expansion not occurring as it failed - I don't think we can silence these as these are from the Rust compiler itself trying to build the code with the failed expansion ? But this is already way better than before :-) Full error
|
ahayzen-kdab
force-pushed
the
38-cxx-gen-errors-do-not-panic
branch
from
feaab7b to
5f7f58b
Compare
|
Hm, I'm currently looking into how CXX generates it's own errors. However, I then still get perfect error messages in rust_analyzer 🤔 I wonder if there's some output from cxx-build that cargo or rust-analyzer can actually understand to provide these nicer diagnostics 🤔 Output if I add a code similar to your example somewhere in cxx-qt-lib: Full build script output |
|
@LeonMatthesKDAB does CXX error or warn ? As note out build script only warns at the moment ? |
|
CXX errors, the build script actually fails. I think this might be the code responsible for doing this: pub fn bridges(rust_source_files: impl IntoIterator<Item = impl AsRef<Path>>) -> Build {
let ref mut rust_source_files = rust_source_files.into_iter();
build(rust_source_files).unwrap_or_else(|err| {
let _ = writeln!(io::stderr(), "\n\ncxxbridge error: {}\n\n", report(err));
process::exit(1);
})
} |
|
It does not panic though, but rather just return a non-zero exit code... |
|
Fun, so what happens if you do the same with |
|
The strange thing is that I don't see this There's also some machinery in this |
|
The build script output isn't always shown in the console as it's an input to cargo. |
|
I wonder if in the bridge code build(rust_source_files).unwrap_or_else(|err| {
let _ = writeln!(io::stderr(), "\n\ncxxbridge error: {}\n\n", report(err));
process::exit(1);
})Where the pub(crate) fn report(error: impl StdError) -> impl Display {
struct Report<E>(E);
impl<E: StdError> Display for Report<E> {
fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
write!(formatter, "{}", self.0)?;
let mut error: &dyn StdError = &self.0;
while let Some(cause) = error.source() {
write!(formatter, "\n\nCaused by:\n {}", cause)?;
error = cause;
}
Ok(())
}
}
Report(error)
} |
|
Hmm but then the cause goes missing and we only get |
|
And it's definitely stopping at |
|
Yup so the source() is None when i try the same code for us, |
|
So, what I've found. More and more I suspect that that rust_analyzer for some reason still continues with macro expansion, even though the build script failed. I just don't know why it does it for CXX, but not for CXX-Qt. |
|
It may be worth reaching out to the rust-analyzer maintainers for advice how to handle this. |
|
WTF... I'm just testing diagnostics on our current main. The output of But that is something that CXX fixes using the codespan-reporting anyway, which we can use as well... See: I do however, also get this error: Which I'm not certain where that's coming from. |
|
It definitely also works with our own errors as well: So maybe we don't need this change and only need to pretty-print our own diagnostics in the build script??? |
|
It even works with diagnostics inside However, they only show up after the file has been saved. Also, it generates a diagnostic on the macro itself: Probably because one of the diagnostic Spans is pointing to output of our macro. |
|
So, I've got the basic diagnostic display going from our build script:
|
ahayzen-kdab
force-pushed
the
38-cxx-gen-errors-do-not-panic
branch
from
5f7f58b to
d072ef9
Compare
ahayzen-kdab
force-pushed
the
38-cxx-gen-errors-do-not-panic
branch
from
d072ef9 to
c94b177
Compare
This allows for CXX errors to appear with a span in the macro expansion
Before this change if you had a unsupported type in an invokable only
the following error would occur.
```
[build] thread 'main' panicked at 'Could not generate C++ from Rust file:
Syn(Error("unsupported type: QUrl"))', crates/cxx-qt-build/src/lib.rs:120:14
```
Now with this change we have an additional error from the macro expansion.
```
[build] error: unsupported type: QUrl
[build] --> examples/qml_minimal/rust/src/cxxqt_object.rs:57:35
[build] |
[build] 57 | pub fn test(&self, _url: &QUrl) {
[build] | ^^^^
```
Related to KDAB#38
ahayzen-kdab
force-pushed
the
38-cxx-gen-errors-do-not-panic
branch
from
c94b177 to
0c1f8a6
Compare
This allows for CXX errors to appear with a span in the macro expansion
Before this change if you had a unsupported type in an invokable only the following error would occur.
Now with this change we have an additional error from the macro expansion.
Note however this does cause loads of extra errors below but this is a side effect of the rest of the CXX macro not being expanded, this first error is the correct one though.
Related to #38