Future proof us against upcoming changes to derives #1787

sgrif · 2018-07-17T15:22:34Z

Macro hygiene used to ignore module scope, which meant that we could
reference a type from outside our wrapper module even if we didn't
import it. That is changing in an upcoming version of Rust.

Unfortunately, we can't just do either of the recommended solutions
(either adding use super::* or changing to use a function instead of a
module). use super::* doesn't work for types defined inside of a
function, and changing ot use a function instead of a module broke our
workaround to pretend we had $crate when we really didn't.

With this change, an item named diesel must be present at the crate
root. In the case of Diesel itself, this is just a module, but for all
users they will now have to put extern crate diesel at the crate root.
It's unlikely folks were renaming it, but we will no longer work if they
do (the import was already located at the crate root, since you cant do
#[macro_use] extern crate anywhere else).

Fixes #1785.

Macro hygiene used to ignore module scope, which meant that we could reference a type from outside our wrapper module even if we didn't import it. That is changing in an upcoming version of Rust. Unfortunately, we can't just do either of the recommended solutions (either adding `use super::*` or changing to use a function instead of a module). `use super::*` doesn't work for types defined inside of a function, and changing ot use a function instead of a module broke our workaround to pretend we had `$crate` when we really didn't. With this change, an item named `diesel` must be present at the crate root. In the case of Diesel itself, this is just a module, but for all users they will now have to put `extern crate diesel` at the crate root. It's unlikely folks were renaming it, but we will no longer work if they do (the import was already located at the crate root, since you cant do `#[macro_use] extern crate` anywhere else). Fixes #1785.

Because we deny warnings, this is breaking our nightly build on CI (which does not fail CI overall). This will only become an issue once these changes migrate to the beta channel in a few weeks. This patch can be reverted as part of upgrading to Diesel 1.4 once that is released. See the [upstream PR] for more information. [upstream PR]: diesel-rs/diesel#1787

Because we deny warnings, this new lint will cause our CI to fail once it hits beta. Allowing `unknown_lints` is necessary because stable-1.28 isn't aware of this lint. This patch can be reverted as part of upgrading to Diesel 1.4 once that is released. See the [upstream PR] for more information. [upstream PR]: diesel-rs/diesel#1787

1469: Allow a new macro hygiene lint until we get Diesel 1.4 r=jtgeibel a=jtgeibel Because we deny warnings, this new lint will cause our CI to fail once it hits beta. Allowing `unknown_lints` is necessary because stable-1.28 isn't aware of this lint. This patch can be reverted as part of upgrading to Diesel 1.4 once that is released. See the [upstream PR] for more information. [upstream PR]: diesel-rs/diesel#1787 Co-authored-by: Justin Geibel <jtgeibel@gmail.com>

* removed some (useless?) output * also ran clippy, note there is a warning about: ``` warning: cannot find type `As` in this scope --> src/oracle/query_builder/alias.rs:17:24 | 17 | #[derive(Debug, Clone, QueryId)] | ^^^^^^^ names from parent modules are not accessible without an explicit import | = note: #[warn(proc_macro_derive_resolution_fallback)] on by default = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue #50504 <rust-lang/rust#50504> ``` which seems to be fixed with diesel-rs/diesel#1787 which has not been released yet

djc · 2018-09-19T08:26:38Z

So these warnings are now also triggered on stable Rust with 1.29.0, with the result that my little Diesel (1.3.3) project on stable Rust now generates pages and pages of warnings on every compile. Could this be backported to an 1.3.x release, or will be 1.4.0 be released soon?

sgrif · 2018-09-20T18:58:47Z

#1785 (comment)

sergeysova · 2018-10-29T23:32:35Z

Does it already release?

weiznich · 2018-10-29T23:55:47Z

@sergeysova Unfortunately not because rustdoc does not build our documentation we cannot do a release currently.

qrilka · 2018-11-05T15:25:54Z

@weiznich is there any issue tracking this rustdoc problem?

weiznich · 2018-11-05T19:00:21Z

@qrilka rust-lang/rust#54524

r-arias · 2018-11-12T20:48:56Z

@weiznich that issue should be resolved in stable 1.30.1 (cargo doc works for me on this version); is there anything else blocking a release?

weiznich · 2018-11-12T21:14:15Z

@r-arias Yes, our ci is failing on windows only. Unfortunately I have no access to a windows machine, so no way for me to track down that issue. Hopefully someone other from @diesel-rs/core (or someone other) with access to a windows machine could help here.

(That said: That error looks quite strange, because it's only happening on windows and the code that causes this according to rustc has no conditional thing based on the OS 😟. I also opened a issue on the rustc repo for this: rust-lang/rust#55832 )

r-arias · 2018-11-13T10:27:24Z

Thanks for the response. Looks like only sqlite causes trouble?

weiznich · 2018-11-13T10:37:01Z

@r-arias Yes only sqlite causes problems.

r-arias · 2018-11-13T10:55:08Z

Could it be that sqlite is wrongly configured in the win environment? Just a guess, I am not familiar with how the test stage is set up...

weiznich · 2018-11-13T10:58:15Z

@r-arias That's not a configuration issue of the environment, because it does not fail at the linking stage, but while compiling the rust code. In detail while trying to resolve some traits. Those traits do not depend on any os specific impls and work on all other platforms. (That's probably a bug in rustc but without windows I'm not able to dig further into this.)

r-arias · 2018-11-13T11:03:17Z

Hmm, I thought maybe the schema is wrongly generated:

2018-11-08T16:12:12.5030063Z error[E0433]: failed to resolve. Could not find `users` in `schema`
2018-11-08T16:12:12.5053093Z  --> diesel_tests\tests\select.rs:8:17
2018-11-08T16:12:12.5055022Z   |
2018-11-08T16:12:12.5055296Z 8 |     use schema::users::dsl::*;
2018-11-08T16:12:12.5059000Z   |                 ^^^^^ Could not find `users` in `schema`

(here)

weiznich · 2018-11-13T11:12:37Z

That would be possible, but how? The same database url is used for the build.rs file to run the migrations, so I do not see how running the migrations would be ok, but running infer_schema on the same url would fail afterwards.

r-arias · 2018-11-13T11:20:00Z

Can't say, I'm just not familiar enough with this :/

r-arias · 2018-11-24T10:19:59Z

Looks like it's building on azure, now?

https://dev.azure.com/diesel-rs/diesel/_build/results?buildId=10

I can't look at appveyor, where it is still failing.

icefoxen · 2018-12-09T19:37:34Z

Sorry for reviving this, but I thought with Rust 2018 #[macro_use] and extern crate ...; were supposed to not be needed anymore. I can't come up with the right combination of use decl's to get all the right bits of the macro in the same scope though. Is there a good way to do this yet, or should we just stick with #[macro_use]?

Thanks a lot.

weiznich · 2018-12-09T19:46:30Z

@icefoxen For supporting the new macro import feature we need to change some internal macros. (The issue is basically that we call internally some macros from the exported ones. Because of the macro hygiene rules a user is not able to provide reference to those internal macros.) Till that is done continue to use #[macro_use].

icefoxen · 2018-12-09T21:26:15Z

Will do, thanks a lot!

jhpratt · 2018-12-19T07:02:19Z

@weiznich Is there a tracking issue for proc macros? I'm unable to find any.

With the "future proof for upcoming Rust hygiene" PR diesel-rs/diesel#1787 Diesel no longer supports renames in the crate root, so testing for that was breaking things unnecessarily.

sgrif requested a review from a team July 17, 2018 15:22

sgrif mentioned this pull request Jul 17, 2018

hygiene: Decouple transparencies from expansion IDs rust-lang/rust#51952

Merged

sgrif merged commit 68d0b9e into master Jul 18, 2018

sgrif deleted the sg-future-proof-imports branch July 18, 2018 19:59

jtgeibel mentioned this pull request Aug 1, 2018

Allow a new macro hygiene lint until we get Diesel 1.4 rust-lang/crates.io#1469

Merged

Eijebong mentioned this pull request Aug 28, 2018

Resolution of macro derive behavior has been modified on 1.30 #1829

Closed

1 task

lthms mentioned this pull request Sep 8, 2018

Update to a more recent rocket and rust toolchain Plume-org/Plume#205

Merged

3 tasks

weiznich mentioned this pull request Dec 1, 2018

table! macros triggers host of deprecation warnings #1928

Closed

2 tasks

cantino mentioned this pull request Jan 15, 2019

Upgrade to Rust 2018 Edition kubos/kubos#256

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Future proof us against upcoming changes to derives #1787

Future proof us against upcoming changes to derives #1787

sgrif commented Jul 17, 2018

djc commented Sep 19, 2018

sgrif commented Sep 20, 2018

sergeysova commented Oct 29, 2018

weiznich commented Oct 29, 2018

qrilka commented Nov 5, 2018

weiznich commented Nov 5, 2018

r-arias commented Nov 12, 2018

weiznich commented Nov 12, 2018

r-arias commented Nov 13, 2018

weiznich commented Nov 13, 2018

r-arias commented Nov 13, 2018

weiznich commented Nov 13, 2018

r-arias commented Nov 13, 2018

weiznich commented Nov 13, 2018

r-arias commented Nov 13, 2018

r-arias commented Nov 24, 2018

icefoxen commented Dec 9, 2018

weiznich commented Dec 9, 2018

icefoxen commented Dec 9, 2018

jhpratt commented Dec 19, 2018

Future proof us against upcoming changes to derives #1787

Future proof us against upcoming changes to derives #1787

Conversation

sgrif commented Jul 17, 2018

djc commented Sep 19, 2018

sgrif commented Sep 20, 2018

sergeysova commented Oct 29, 2018

weiznich commented Oct 29, 2018

qrilka commented Nov 5, 2018

weiznich commented Nov 5, 2018

r-arias commented Nov 12, 2018

weiznich commented Nov 12, 2018

r-arias commented Nov 13, 2018

weiznich commented Nov 13, 2018

r-arias commented Nov 13, 2018

weiznich commented Nov 13, 2018

r-arias commented Nov 13, 2018

weiznich commented Nov 13, 2018

r-arias commented Nov 13, 2018

r-arias commented Nov 24, 2018

icefoxen commented Dec 9, 2018

weiznich commented Dec 9, 2018

icefoxen commented Dec 9, 2018

jhpratt commented Dec 19, 2018