Skip to content

Allow specifying a module path to the phf crate for codegen#81

Merged
sfackler merged 1 commit intorust-phf:masterfrom
aidanhs:aphs-set-phf-path
Apr 25, 2016
Merged

Allow specifying a module path to the phf crate for codegen#81
sfackler merged 1 commit intorust-phf:masterfrom
aidanhs:aphs-set-phf-path

Conversation

@aidanhs
Copy link
Copy Markdown
Contributor

@aidanhs aidanhs commented Feb 28, 2016

At the most basic level, this means you can use phf_codegen if you've got something like extern crate phf as __some_phf_alias;. More usefully, this PR permits crates to add their own codegen on top of phf without requiring all consuming crates to explicitly depend on phf.

Concretely, I want this for servo/string-cache#136. See https://github.com/servo/string-cache/pull/136/files#diff-f5597aa77bf5eea303399f7125c5a0adR10 and the current hack I'm resorting to at https://github.com/servo/string-cache/pull/136/files#diff-141f99248469f8dc2a1daf108bbf23c1R71 (relies on the next string being written to start with ::phf).

To briefly explain the problem: it's a given that consumers of string_cache_codegen will need to depend on string_cache. However, if string_cache_codegen generates a vanilla phf map, consumers of string_cache_codegen also need to add phf to their own Cargo.toml and add extern crate phf;. Exposing phf under the string_cache crate sidesteps this inconvenience by using the existing string_cache dependency (and means you don't have to upgrade string_cache and phf in lockstep).

(I'm open to suggestions for better ways to do this by the way)

Comment thread phf_codegen/src/lib.rs Outdated
/// A builder for the `phf::Set` type.
pub struct Set<T> {
map: Map<T>,
path: String,
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.

You can probably just look at the path of the Map instead of also tracking this here.

@sfackler
Copy link
Copy Markdown
Collaborator

Hey, sorry for missing this until now. Seems reasonable other than one comment.

@aidanhs aidanhs force-pushed the aphs-set-phf-path branch from a555b51 to 4a7999e Compare April 25, 2016 20:02
@aidanhs
Copy link
Copy Markdown
Contributor Author

aidanhs commented Apr 25, 2016

Rebased and made suggested change

@sfackler
Copy link
Copy Markdown
Collaborator

Thanks!

@sfackler sfackler merged commit c59f70e into rust-phf:master Apr 25, 2016
@aidanhs aidanhs deleted the aphs-set-phf-path branch April 25, 2016 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants