Fail to build with newer rustc

[fulminemizzega]

Hello, can you update the lexical-core dependency to 0.7.6? With rustc 1.66.0 (69f9c33d7 2022-12-12) I get this error:

Some errors have detailed explanations: E0277, E0308.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `lexical-core` due to 27 previous errors

With updated lexical-core it compiles but 2 cargo tests fail:

failures:

---- diagnostics::tests::test_diagnostics stdout ----
thread 'diagnostics::tests::test_diagnostics' panicked at 'assertion failed: `(left == right)`
  left: `PublishDiagnosticsParams { uri: Url { scheme: "file", host: None, port: None, path: "/home/edo/.local/share/nvim/veridian/test_data/diag/diag_test.sv", query: None, fragment: None }, diagnostics: [], version: None }`,
 right: `PublishDiagnosticsParams { uri: Url { scheme: "file", host: None, port: None, path: "/home/edo/.local/share/nvim/veridian/test_data/diag/diag_test.sv", query: None, fragment: None }, diagnostics: [Diagnostic { range: Range { start: Position { line: 3, character: 13 }, end: Position { line: 3, character: 13 } }, severity: Some(Error), code: None, code_description: None, source: Some("slang"), message: " cannot refer to element 2 of 'logic[1:0]'", related_information: None, tags: None, data: None }], version: None }`', src/diagnostics.rs:252:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- diagnostics::tests::test_verible_syntax stdout ----
thread 'diagnostics::tests::test_verible_syntax' panicked at 'called `Option::unwrap()` on a `None` value', src/diagnostics.rs:285:74

failures:
    diagnostics::tests::test_diagnostics
    diagnostics::tests::test_verible_syntax

test result: FAILED. 19 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.09s

I also tried to update everything and run cargo test --all-features, there is only one failing:

failures:

---- diagnostics::tests::test_verible_syntax stdout ----
thread 'diagnostics::tests::test_verible_syntax' panicked at 'called `Option::unwrap()` on a `None` value', src/diagnostics.rs:285:74
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    diagnostics::tests::test_verible_syntax

test result: FAILED. 20 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s

Thanks

[vivekmalneedi]

I'm not sure exactly why but as of #179 it seems to be building on newer rust versions, I just tested it on rust 1.74.1

[fulminemizzega]

Thanks! I've just tested with rustc 1.75.0 (82e1608df 2023-12-21)

cargo install --git https://github.com/vivekmalneedi/veridian.git --all-features

and it installs fine in a fedora-minimal container (it needs clang-libs for libclang.so and openssl-devel to build). I tried again also running tests (after reading my previous message) with cargo test --all-features after cloning the repo:

failures:

---- diagnostics::tests::test_verible_syntax stdout ----
thread 'diagnostics::tests::test_verible_syntax' panicked at src/diagnostics.rs:298:74:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    diagnostics::tests::test_verible_syntax

test result: FAILED. 20 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

Would it be better to track this feature in a new issue and close this one?

[vivekmalneedi]

that test is for syntax checking with verible so if that isn't installed it will fail. I guess it could show a nicer error message but it's just a test so I haven't bothered.

[fulminemizzega]

I do not know any rust... is something like this acceptable?

        let errors = verible_syntax(&doc, "verible-verilog-syntax", &[])
            .expect("verible-verilog-syntax not found, test not run");

reference https://github.com/vivekmalneedi/veridian/blob/87bb5282f46aa637825b08adcde3e094f6544cf6/src/diagnostics.rs#L298

[vivekmalneedi]

yes, that would work as a better error message, ideally we would be able to skip the test if verible wasn't present but I don't think that functionality exists in the cargo test runner.