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 grammar in docs for {f32,f64}::from_str, mention known bug. #56217

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@frewsxcv
Member

frewsxcv commented Nov 25, 2018

  • Original bug about documenting grammar
  • Known bug with parsing
/// ((e|E)
/// (\+|-)?
/// [0-9]+)?)
/// ```

This comment has been minimized.

@frewsxcv

frewsxcv Nov 25, 2018

Member

I'm not tied to the particular formatting of the regular expression here. You can visualize this as a railroad diagram here.

This comment has been minimized.

@frewsxcv

frewsxcv Nov 25, 2018

Member

Also if people would prefer, I can migrate this to BNF, if people think that'd be easier for readers

This comment has been minimized.

@nagisa

nagisa Nov 25, 2018

Contributor

We could embed SVG of the railroad in-line… would look nice but barely maintainable.

This comment has been minimized.

@nagisa
@frewsxcv

This comment has been minimized.

Member

frewsxcv commented Nov 25, 2018

/// [0-9]+)?)
/// ```
///
/// # Known bugs

This comment has been minimized.

@frewsxcv

frewsxcv Nov 25, 2018

Member

Historically, we haven't mentioned known bugs in the docs, but considering this known bug has existed for years and people keep hitting it, I think it's worth calling it out here.

@frewsxcv frewsxcv changed the title from Add grammar for {f32,f64}::from_str, mention known bug. to Add grammar in docs for {f32,f64}::from_str, mention known bug. Nov 25, 2018

@frewsxcv frewsxcv force-pushed the frewsxcv:frewsxcv-float-parse branch from b8b884c to d1c1978 Nov 25, 2018

Add grammar for {f32,f64}::from_str, mention known bug.
- Original bug about documenting grammar
  - #32243
- Known bug with parsing
  - #31407

@frewsxcv frewsxcv force-pushed the frewsxcv:frewsxcv-float-parse branch from d1c1978 to 1798d51 Nov 25, 2018

@rust-lang rust-lang deleted a comment from rust-highfive Nov 25, 2018

@rust-lang rust-lang deleted a comment from rust-highfive Nov 25, 2018

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 27, 2018

FWIW this isn't the first instance of documenting a known issue, and I think while we don't really have a policy around this the defacto policy is that "if someone cares enough to document the issue while not being willing to fix it, then seems fine to document".

@QuietMisdreavus

This comment has been minimized.

Member

QuietMisdreavus commented Dec 3, 2018

I'm not a fan of including a grammar or regular expression like that, especially in the std docs. We've gradually been moving any kind of grammar into the Reference, so i would consider that a much better place for that.

As far as documenting known issues, i'm okay with it, but i think it could be better phrased:

# Known issues

In some situations, some strings that should create a valid float instead return an
error. See [issue #31407] for details.
@frewsxcv

This comment has been minimized.

Member

frewsxcv commented Dec 3, 2018

I'm not a fan of including a grammar or regular expression like that, especially in the std docs.

The point of this grammar is to document valid input formats for this particular standard library API. So I can get a better understanding of what you're envisioning, do you have any examples of sections in the Reference that document specific standard library APIs?

We've gradually been moving any kind of grammar into the Reference

Moving the Rust language syntax grammar to the Reference makes sense to me since it's documenting the behaviors of the language itself, as opposed to a specific standard library API

but i think it could be better phrased:

Thanks for the suggestion! I'll update it to reflect your wording

frewsxcv added some commits Dec 3, 2018

@QuietMisdreavus

This comment has been minimized.

Member

QuietMisdreavus commented Dec 3, 2018

do you have any examples of sections in the Reference that document specific standard library APIs?

There's a "Special types and traits" page that seems to document some of the more-common items from the standard library.

But i think i get your point. Since this is library code, it makes sense to document it here. (Though i'm not sure if we ever got together a better documentation of all type conversion functionality than the std::convert module docs, and that doesn't even include the FromStr trait being documented here...)

There is a grammar for floating-point literals in the reference; does this match the regular expression you've entered there? I guess it wouldn't include the type suffix, but i think everything else matches?

@frewsxcv

This comment has been minimized.

Member

frewsxcv commented Dec 16, 2018

but i think everything else matches?

A few other differences come to mind:

  • float literals, like integer literals, can have underscores
  • float literals require a decimal point
  • float literals can't have a leading +
@frewsxcv

This comment has been minimized.

Member

frewsxcv commented Dec 16, 2018

Also looks like float literals can have a capital E in the exponent part

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment