auto-impl-rs / auto_impl

Automatically implement traits for common smart pointers
Apache License 2.0
104 stars 14 forks source link

Use trybuild and move to proc-macro-error #60

Closed CreepySkeleton closed 4 years ago

CreepySkeleton commented 4 years ago

This PR is on top of #59

Deleted files

I also added helpful notes where applicable, made some refactoring here and there (not much), fixed some typos.

I didn't touch some panic! invocations - they look like bug catching and this is just the job of panic. Some other panics were replaced with abort!.

Btw, nightly feature from proc_macro2 doesn't really do anything, I suggest to remove it.

Also, I would like to propose not to use Trybuid::compile_pass - it literally executes a separate cargo build run for every each file which is slow.

CreepySkeleton commented 4 years ago

And in general, please only include style changes that are justified by the official Rust styleguide (i.e. what rustfmt does).

I didn't do any style adjustments except that one match () => if, the blame is on rustfmt entirely :)

CreepySkeleton commented 4 years ago

Here's an attempt to integrate rustfmt and make it match your code style. Unfortunately, those options are nightly, but who cares?

CreepySkeleton commented 4 years ago

Of course it would be fine to use abort! only but in most cases it's quite useful to keep working, looking for other errors to report. I tweaked the code in a way that functions return as late as possible, take a look.

CreepySkeleton commented 4 years ago

knock knock @LukasKalbertodt

LukasKalbertodt commented 4 years ago

@CreepySkeleton Please excuse the delay, I have plenty of other stuff to do :( But I have not forgotten this and planned on completely reviewing this PR this week still!

CreepySkeleton commented 4 years ago

Ugh, I must admit I completely forgot about this PR, sorry about it @LukasKalbertodt :( In my defense, nobody tried to ping me anyway.

I fixed all of the suggestions above If you sill have an appetite for this changes.

LukasKalbertodt commented 4 years ago

No problem for the delay ;-) Thanks for fixing everything! Great work with this PR.