Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions dropshot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ indexmap = "1.0.0"
openapiv3 = "0.5.0"
paste = "1.0.0"
percent-encoding = "2.1.0"
proc-macro2 = "1.0.27"
serde_json = "1.0.0"
serde_urlencoded = "0.7.0"
slog-async = "2.4.0"
Expand Down
29 changes: 29 additions & 0 deletions dropshot/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use http::StatusCode;
use percent_encoding::percent_decode_str;
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::sync::Once;

/**
* `HttpRouter` is a simple data structure for routing incoming HTTP requests to
Expand Down Expand Up @@ -180,10 +181,38 @@ impl<'a> From<&'a str> for InputPath<'a> {
}
}

static INFORM_PROC_MACRO2_WE_ARE_NOT_A_PROC_MACRO: Once = Once::new();

/*
* Validate that the string is a valid Rust identifier.
*/
fn valid_identifier(var: &str) -> bool {
// TODO: Remove this "Once" callback when the following issue
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

FWIW @davepacheco has tried (and failed) to hold the line on /* */ comments in dropshot so... do what you will with that .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I always considered that comment style "valid, but non-idiomatic" - the Rust book only shows the "//" style (see: here) and there was even an (admittedly unsuccessful) attempt to remove block comments: rust-lang/rfcs#1371

(Plus I heard a rustlang maintainer also had the spicy take that they're not idiomatic)

Do you want me to switch before landing? I noticed we're pretty inconsistent across many of Oxide's repos (dunno if we should clarify which repos do / don't have this styling more explicitly).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The rustdoc book also exclusively uses the non-block comment style ("//" or "///")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know.

// is resolved.
// - https://github.com/alexcrichton/proc-macro2/issues/218
//
// proc_macro2 tries checking if the calling code exists within a proc_macro
// (as opposed to, e.g., a library/binary) by installing a custom panic
// handler (!), invoking it, and checking the result to see if the
// compiler's implementation of some proc macro functionality is available.
// - If it is available: proc_macro2 uses the compiler's codebase.
// - If it isn't: proc_macro uses a fallback implementation.
//
// For a program compiled with "panic = unwind", this admittedly works
// (... although it's arguably questionable to overwrite the panic hook),
// but it causes a program compiled with "panic = abort" to die.
//
// To workaround this, we force proc_macro2 to use the fallback
// implementation and avoid doing this panic-hook-based checking.
//
// As documented in the aforementioned issue, dtolnay plans on removing
// the panic-hook-based check once a compiler feature stabilizes
// to provide a better way of doing the check: proc_macro::is_available.
//
// Once that's done, we can remove this hack on our side.
INFORM_PROC_MACRO2_WE_ARE_NOT_A_PROC_MACRO.call_once(|| {
proc_macro2::fallback::force();
});
syn::parse_str::<syn::Ident>(var).is_ok()
}

Expand Down