Commit Graph

16 Commits

Author SHA1 Message Date
Nick Mathewson 932fe48eaf Run add_warnings. 2022-11-03 11:06:02 -04:00
Ian Jackson f84d8777db cargo fmt to remove blank lines
Apparently cargo fmt doesn't like these, which my perl rune didn't
delete.

This commit is precisely the result of `cargo fmt`.
2022-10-12 15:29:04 +01:00
Ian Jackson d9910dba08 Replace all README copies in src/lib.rs with includes
The feature we want is `#[doc = include_str!("README.md")]`, which is
stable since 1.54 and our MSRV is now 1.56.

This commit is precisely the result of the following Perl rune:
  perl -i~ -0777 -pe 's{(^//!(?!.*\@\@).*\n)+}{#![doc = include_str!("../README.md")]\n}m' crates/*/src/lib.rs
2022-10-12 15:29:03 +01:00
trinity-1686a 7f939fa480 enable doc_auto_cfg feature on every crate when documenting for docs.rs 2022-08-24 18:22:41 +02:00
Ian Jackson 589c6e52bb Run maint/add_warning crates/*/src/{lib,main}.rs
Update all lint blocks
2022-06-23 19:15:42 +01:00
Nick Mathewson bf2336e547 Style fixes to tor-events errors. 2022-06-22 08:39:44 -04:00
Ian Jackson 4f42101554 lints: Add let_unit_value allow to all crates
From running add_warning, with manual picking of the right
hunks/lines.
2022-05-31 15:23:52 +01:00
Ian Jackson ba0843da4a lints: Add lint block delimiters to every crate
This was the result of:
  maint/add_warning crates/*/src/{lib,main}.rs
and then manually curating the results.
2022-05-31 13:00:31 +01:00
Nick Mathewson 546ae3000e Resolve the new `derive_partial_eq_without_eq` lint.
It's a little overzealous sometimes, but it's mostly to the good.
2022-05-23 12:55:37 -04:00
Ian Jackson afb50fe735 Disable clippy::clone_on_ref_ptr
This lint is IMO inherently ill-conceived.

I have looked for the reasons why this might be thought to be a good
idea and there were basically two (and they are sort of contradictory):

I. "Calling ‘.clone()` on an Rc, Arc, or Weak can obscure the fact
    that only the pointer is being cloned, not the underlying data."

This is the wording from
  https://rust-lang.github.io/rust-clippy/v0.0.212/#clone_on_ref_ptr

It is a bit terse; we are left to infer why it is a bad idea to
obscure this fact.  It seems to me that if it is bad to obscure some
fact, that must be because the fact is a hazard.  But why would it be
a hazard to not copy the underlying data ?

In other languages, faliing to copy the underlying data is a serious
correctness hazard.  There is a whose class of bugs where things were
not copied, and then mutated and/or reused in multiple places in ways
that were not what the programmer intended.  In my experience, this is
a very common bug when writing Python and Javascript.  I'm told it's
common in golang too.

But in Rust this bug is much much harder to write.  The data inside an
Arc is immutable.  To have this bug you'd have use interior mutability
- ie mess around with Mutex or RefCell.  That provides a good barrier
to these kind of accidents.

II. "The reason for writing Rc::clone and Arc::clone [is] to make it
     clear that only the pointer is being cloned, as opposed to the
     underlying data. The former is always fast, while the latter can
     be very expensive depending on what is being cloned."

This is the reasoning found here
  https://github.com/rust-lang/rust-clippy/issues/2048

This is saying that *not* using Arc::clone is hazardous.
Specifically, that a deep clone is a performance hazard.

But for this argument, the lint is precisely backwards.  It's linting
the "good" case and asking for it to be written in a more explicit
way; while the supposedly bad case can be written conveniently.

Also, many objects (in our codebase, and in all the libraries we use)
that are Clone are in fact simply handles.  They contain Arc(s) (or
similar) and are cheap to clone.  Indeed, that is the usual case.

It does not make sense to distinguish in the syntax we use to clone
such a handle, whether the handle is a transparent Arc, or an opaque
struct containing one or more other handles.

Forcing Arc::clone to be written as such makes for code churn when a
type is changed from Arc<Something> to Something: Clone, or vice
versa.
2022-02-24 18:15:44 +00:00
Nick Mathewson 1cecc7e45a Change deny(clippy::all) to warn(clippy::all).
Closes #338.
2022-02-14 09:24:06 -05:00
Daniel Eades 592642a9e6 extend lints to include 'clippy::all' 2021-12-28 20:15:40 +00:00
Daniel Eades db16d13df4 add semicolons if nothing returned 2021-11-25 13:20:37 +00:00
Nick Mathewson 54de7f5cfd Remove a couple more eprintln! calls. 2021-11-23 17:40:13 -05:00
Nick Mathewson 9479001649 Add and resolve clippy warnings in tor-events.
Here we add the same array of clippy warnings as usual to the new
tor-event crate, and resolve the issues that triggered any of them.
2021-11-22 11:49:31 -05:00
eta 98f38dc186 Initial cut at a typed event framework for arti (arti#230).
This implements a basic typed event broadcast mechanism, as described in
arti#230: consumers of the new `tor-events` crate can emit `TorEvent`
events, which others can consume via the `TorEventReceiver`.

Under the hood, the crate uses the `async-broadcast`
(https://github.com/smol-rs/async-broadcast) crate, and a
`futures::mpsc::UnboundedSender` for the event emitters; these are glued
together in the `EventReactor`, which must be run in a background thread
for things to work. (This is done so event sending is always cheap and
non-blocking, since `async-broadcast` senders don't have this
functionality.)

Additionally, the `TorEventKind` type is used to implement selective
event reception / emission: receivers can subscribe to certain event
types (and in fact start out receiving nothing), which filters the set
of events they receive. Having no subscribers for a given event type
means it won't even be emitted in the first place, making things more
efficient.
2021-11-22 14:39:38 +00:00