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.

Sign up for GitHub

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

Jump to bottom

Make rustdoc JSON not cause ICE #3190

Merged
merged 2 commits into from
May 31, 2022
Merged

Make rustdoc JSON not cause ICE #3190

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e01025f
Make rustdoc JSON not cause ICE
Enselic May 26, 2022
510cad6
Also replace `Self` with `T` for `impl<T: Query> AsQuery for T`
Enselic May 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions diesel/src/expression/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ where
T: Expression<SqlType = ST>,
ST: SqlType + TypedExpressionType,
{
type Expression = Self;
type Expression = T;

fn as_expression(self) -> Self {
fn as_expression(self) -> T {
self
}
}
Expand Down
9 changes: 0 additions & 9 deletions diesel/src/query_builder/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,15 +332,6 @@ pub trait AsQuery {
fn as_query(self) -> Self::Query;
}

impl<T: Query> AsQuery for T {
type SqlType = <Self as Query>::SqlType;
type Query = Self;

fn as_query(self) -> Self::Query {
self
}
}

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for working on this ❤️

This change would unfortunately break most of diesel's query builder. As a blind guess I would expect that this is also the main fix for the ICE, as this impl implies quite a lot of other concrete impls. (Just have a look what else implements AsQuery or Query to get a feeling for that). If I can help with concrete information here just reach out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look! I was just being a bit lazy. I pushed a commit where I did the same change as I did for AsExpression, namely replacing Self with T, and now the diff should look less problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@weiznich If you are familiar with the diesel code base, the best way you can help is probably to remove more and more code from diesel/diesel until the ICE stops from happening. Do it "one line at a time". Then we will know what line that is crucial to making the ICE happen. I have tried this myself but I found the diesel code base to be riddled with dependencies spread across the code base via macros and such, and I wasn't tenacious enough to follow through. But finding a minimal way to reproduce the problem we see would certainly help. That is what rust-lang/rust#93588 (comment) was talking about too.

Once have such an MRE it will probably not be hard to fix the root problem.

/// Takes a query `QueryFragment` expression as an argument and returns a type
/// that implements `fmt::Display` and `fmt::Debug` to show the query.
///
Expand Down
4 changes: 3 additions & 1 deletion diesel/src/query_dsl/load_dsl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,15 @@ pub trait ExecuteDsl<Conn: Connection<Backend = DB>, DB: Backend = <Conn as Conn
fn execute(query: Self, conn: &mut Conn) -> QueryResult<usize>;
}

use crate::result::Error;

impl<Conn, DB, T> ExecuteDsl<Conn, DB> for T
where
Conn: Connection<Backend = DB>,
DB: Backend,
T: QueryFragment<DB> + QueryId,
{
fn execute(query: Self, conn: &mut Conn) -> QueryResult<usize> {
fn execute(query: T, conn: &mut Conn) -> Result<usize, Error> {
conn.execute_returning_count(&query)
}
}
Expand Down