Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a ValueParser #11

Open
ndmitchell opened this issue Mar 18, 2021 · 0 comments
Open

Add a ValueParser #11

ndmitchell opened this issue Mar 18, 2021 · 0 comments

Comments

@ndmitchell
Copy link
Contributor

Broken out from #7 and other discussions. Currently, values are "parsed" using UnpackValue, which for type T is fn(Value) -> Option<T>. That sucks because it doesn't say what you wanted or what didn't match. Imagine T is Vec<i32>, and the value is [1,"test"]. It should say the failure arises from "test" not being an Int at the 2nd element of the list. That means the return type should be a result with the error as Unexpected:

enum PathElement {
    Index(i32),
    Field(String),
}
struct Unexpected {
    path: Vec<PathElement>, // The path I followed, e.g. index 1
    expected: String, // What I wanted, e.g. Int
    got: Value, // What I got, e.g. "test"
}

We want this to be the ubiquitous method of getting data from inside Value, to ensure it's used everywhere, and thus we get good error messages always. That requires making it the simplest method and providing good support for this by default - we want manually unpacking a value to be more work.

That means the Unexpected type should be easy to construct, and the ValueParser trait should do it automatically for Vec etc.

We probably also want to be able to generate documentation from ValueParser saying what type the value expects in advance, to be used for auto-documentation and function signatures. That gives us approx:

trait ValueParser {
     fn parse(x: Value) -> Result<Self, Unexpected>;
     fn sig() -> String;
}
facebook-github-bot pushed a commit that referenced this issue Jan 9, 2024
Summary: Pull Request resolved: facebookexperimental/allocative#11

Reviewed By: ndmitchell

Differential Revision: D52630519

Pulled By: stepancheg

fbshipit-source-id: 0ce683c4cdcdf00dde904a2368aa2032e571b513
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

No branches or pull requests

1 participant