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

Fixes to build on PR #1

Open
merged 3 commits into
base: clean_clippy_remove_try
Choose a base branch
from
May 2, 2018

Conversation

neoeinstein
Copy link

Fixes the macro build error, some hidden build errors with lossy conversions, and an unnecessary format!() that previously existed.

It doesn't appear that `:tt` accepts the `stringify!()`-ed value in this
position. The :tt is only later used as an `:expr` to produce the name
for metadata purposes.

Converting this position to be an `:expr` allows the `stringify!()`-ed
value and accepts all current uses of the `graphql_scalar!()` macro in
this repository.
The conversions in this changeset cannot use the `From<T>` trait
implementation because the conversion is lossy, either because they
involve converting a signed value to an unsigned value (`i32`⇒`u64`)
or because the convert from a larger data type to a smaller one
(`u64`⇒`i32`). In this case, the `as` type cast is necessary to perform
a bitwise conversion.

This coercion can cause negative values to become very large unsigned
values. This is intentional on line 90.
The `E::custom()` function requires a value of any type that implements
`fmt::Display`, so a plain `&str` works just fine here.
@theduke theduke merged commit 3685a21 into Atul9:clean_clippy_remove_try May 2, 2018
@theduke theduke force-pushed the clean_clippy_remove_try branch from 3685a21 to 5fad1c4 Compare May 2, 2018 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants