Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to overwrite returned type #733

Merged
merged 3 commits into from Mar 3, 2019

Conversation

Projects
None yet
3 participants
@GuillaumeGomez
Copy link
Member

commented Feb 28, 2019

Fixes #729.

cc @sdroege @EPashkin

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

The error is:

$ if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then rustup component add clippy; cargo clippy --release; fi
error: component 'clippy' for target 'x86_64-unknown-linux-gnu' is unavailable for download
error: 'cargo-clippy' is not installed for the toolchain 'nightly-x86_64-unknown-linux-gnu'
To install, run `rustup component add clippy`
The command "if [ "$TRAVIS_RUST_VERSION" == "nightly" ]; then rustup component add clippy; cargo clippy --release; fi" exited with 1.
@sdroege

This comment has been minimized.

Copy link
Member

commented Feb 28, 2019

Use stable clippy?

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

I didn't change it and I won't (at least no in this PR).

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

Oh crap, forgot to add documentation though!

Show resolved Hide resolved src/analysis/return_value.rs Outdated

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:overwrite-return-type branch from 4022647 to 21191c3 Feb 28, 2019

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

Updated.

.filter_map(|f| f.ret.type_name.as_ref())
.map(|typ| env.library.find_type(0, typ))
.next() {
Some(Some(typ)) => typ,

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 1, 2019

Member

To remove double Some, please use .and_then, and maybe if let instead match

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Mar 3, 2019

Author Member

I don't see how .and_then would improve the situation. Another .filter_map could though but then we'd look to the next arg and I don't want that so I think the only improvement possible is to use if let as you suggested.

This comment has been minimized.

Copy link
@EPashkin

EPashkin Mar 3, 2019

Member

I thought about something like this:

    let typ = configured_functions.iter()
        .filter_map(|f| f.ret.type_name.as_ref())
        .next()
        .and_then(|type_name| env.library.find_type(0, type_name))
        .unwrap_or_else(|| override_string_type_return(env, func.ret.typ, configured_functions));

This comment has been minimized.

Copy link
@GuillaumeGomez

GuillaumeGomez Mar 3, 2019

Author Member

Oh indeed! Didn't think about it this way...

@GuillaumeGomez GuillaumeGomez force-pushed the GuillaumeGomez:overwrite-return-type branch from 94bbdcf to 658bdc7 Mar 3, 2019

@EPashkin

This comment has been minimized.

Copy link
Member

commented Mar 3, 2019

Rustfmt not agree with your formating and with my too 😉
Anyway, please merge PR yourself when CI ended, I will be off for hour-two.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member Author

commented Mar 3, 2019

rustfmt is the enemy of nice code! (we should enforce it at some point too...)

@GuillaumeGomez GuillaumeGomez merged commit 26ffa3b into gtk-rs:master Mar 3, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:overwrite-return-type branch Mar 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.