Skip to content

anyhow Error#398

Merged
bors[bot] merged 1 commit into
masterfrom
anyhow
Apr 23, 2020
Merged

anyhow Error#398
bors[bot] merged 1 commit into
masterfrom
anyhow

Conversation

@burrbull
Copy link
Copy Markdown
Member

@burrbull burrbull commented Nov 14, 2019

@rust-highfive
Copy link
Copy Markdown

rust-highfive commented Nov 14, 2019

r? @Emilgardis

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Nov 14, 2019
@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 14, 2019

cc @therealprof

Looks like it works:

+ /home/burrbull/builds/svd2rust/release/svd2rust --target riscv -i k210.svd
ERROR svd2rust: In peripheral `FFT`
ERROR svd2rust: caused by: In register `ctrl`
ERROR svd2rust: caused by: Parsing field #0
ERROR svd2rust: caused by: In field `point`
ERROR svd2rust: caused by: Expected content in <enumeratedValues> tag, found none
ERROR svd2rust: backtrace: disabled backtrace

@therealprof
Copy link
Copy Markdown
Contributor

therealprof commented Nov 14, 2019

Sweet!

Emilgardis
Emilgardis reviewed Nov 15, 2019
View reviewed changes
Comment thread src/lib.rs
Comment thread Cargo.toml Outdated

[dependencies.svd-parser]
git = "https://github.com/rust-embedded/svd"
branch = "anyhow"
Copy link
Copy Markdown
Member

@Emilgardis Emilgardis Nov 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this is just for the draft.

We could use the new git+version support added for publishing crates added recently, but I'm not sure myself how to properly use it.

That would also not be backwards compatible

Comment thread src/main.rs Outdated
.context("couldn't open the SVD file")?
.read_to_string(xml)
.chain_err(|| "couldn't read the SVD file")?;
.context("couldn't read the SVD file")?;
Copy link
Copy Markdown
Member

@Emilgardis Emilgardis Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this when implementing, should probably be uppercase "C"

Copy link
Copy Markdown
Member

@Emilgardis Emilgardis Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot vs couldn't vs can not

I prefer cannot for this

Comment thread src/main.rs Outdated
.lock()
.read_to_string(xml)
.chain_err(|| "couldn't read from stdin")?;
.context("couldn't read from stdin")?;
Copy link
Copy Markdown
Member

@Emilgardis Emilgardis Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

Comment thread src/generate/peripheral.rs Outdated

let cluster_size = cluster_size_in_bits(cluster, &defs)
.chain_err(|| format!("Cluster {} has no determinable `size` field", cluster.name))?;
.context(format!("Cluster {} has no determinable `size` field", cluster.name))?;
Copy link
Copy Markdown
Member

@Emilgardis Emilgardis Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use with_context

Copy link
Copy Markdown
Member

@Emilgardis Emilgardis Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think there are some other places in rust-embedded/svd#98 that could do the same, but I'm sure it doesnt really matter in optimized builds.

@Emilgardis
Copy link
Copy Markdown
Member

Emilgardis commented Nov 16, 2019 •
edited
Loading

We can merge this as is, if we do we'd bump the required cargo version to publish, but not to compile (published) crate.

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 16, 2019

First we need make new release of svd-parser to remove git dependency.

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 16, 2019

By the way what MSRV version now? Can we merge #381 ?

@therealprof
Copy link
Copy Markdown
Contributor

therealprof commented Nov 16, 2019

@burrbull It changes the MSRV of the PACs to 1.37? I don't think we want to do that just yet. Bumping the MSRV of svd or svd2rust would be fine.

@burrbull burrbull marked this pull request as ready for review November 16, 2019 20:34
@burrbull burrbull requested a review from a team as a code owner November 16, 2019 20:34
@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 16, 2019

bors try

bors Bot added a commit that referenced this pull request Nov 16, 2019
@bors
Try #398:
63d7f41
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Nov 16, 2019

try

Build failed

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 16, 2019

bors try

bors Bot added a commit that referenced this pull request Nov 16, 2019
@bors
Try #398:
a2ba2cc
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Nov 16, 2019

try

Build failed

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 17, 2019

std::backtrace::Backtrace still unstable.

@burrbull burrbull added the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. label Nov 17, 2019
@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 17, 2019

bors try

bors Bot added a commit that referenced this pull request Nov 17, 2019
@bors
Try #398:
4655c89
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Nov 17, 2019

try

Build succeeded

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Nov 17, 2019

@therealprof

Now it works on stable, but without backtrace.

If compile svd2rust on nightly, backtrace works too.

@therealprof
Copy link
Copy Markdown
Contributor

therealprof commented Apr 22, 2020

@burrbull Can you rebase this, fix the conflicts and add a ChangeLog entry?

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Apr 23, 2020

Looks like std::backtrace::Backtrace not stable yet.

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Apr 23, 2020

Rebased

@burrbull
Copy link
Copy Markdown
Member Author

burrbull commented Apr 23, 2020

bors try

bors Bot added a commit that referenced this pull request Apr 23, 2020
@bors
Try #398:
6771c10
@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Apr 23, 2020

try

Build succeeded:

Emilgardis
Emilgardis approved these changes Apr 23, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!
bors r+

@bors
Copy link
Copy Markdown
Contributor

bors Bot commented Apr 23, 2020

Build succeeded:

@bors bors Bot merged commit f7576a9 into master Apr 23, 2020
@bors bors Bot deleted the anyhow branch April 23, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Emilgardis Emilgardis Emilgardis approved these changes

Assignees

@Emilgardis Emilgardis

Labels

S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@burrbull @rust-highfive @therealprof @Emilgardis