Opened 4 years ago

Last modified 2 years ago

#9573 new bug

Add warning for invalid digits in integer literals

Reported by: vlopez Owned by: vlopez
Priority: normal Milestone:
Component: Compiler (Parser) Version: 7.9
Keywords: report-impact, parsing, integer, octal, binary, hexadecimal, Cc: Iceland_jack, carter, hvr
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect warning at compile-time Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by vlopez)

In its latest version, GHC can parse binary (with -XBinaryLiterals), octal and hexadecimal literals:

> 0b101010
> 0o52
> 0x2A

Currently, the parser/lexer reads digits from the input as long as they are valid for the specified radix. All subsequent digits are interpreted as a new, separate token.

If the user uses a digit which isn't valid for the radix, it may be reported with a non-obvious error message, or interpreted in surprising ways:

> :t 0o567
0o576 :: Num a => a
> :t 0o5678
0o5678 :: (Num (a -> t), Num a) => t
> :t 0x1bfah

<interactive>:1:7: Not in scope: h
> replicate 0o5678                                                              
 [8,8,8,8,8,8,8...

We suggest warning the user when a literal of this sort is written, while respecting any other error messages and the original behaviour.

More specifically, the parser or lexer would give a warning if a token starting with an alphanumeric character is found immediately after a numeric literal, without a blank between them.

Attachments (1)

invalid-digit-warning-6c9246.patch (3.9 KB) - added by vlopez 4 years ago.

Download all attachments as: .zip

Change History (11)

Changed 4 years ago by vlopez

comment:1 Changed 4 years ago by vlopez

Here's a partial patch (attachment invalid-digit-warning-6c9246.patch), which gives a warning in most cases.

However, the feature not very useful when the user for making sense of a type error, as GHC only seems to display warnings when the module can be compiled correctly in the first place.

Last edited 4 years ago by vlopez (previous) (diff)

comment:2 Changed 4 years ago by vlopez

Description: modified (diff)
Owner: set to vlopez

comment:3 Changed 4 years ago by andreas.abel

I think a parse *error* would be more appropriate (and useful) than a warning. I consider it as a bug rather than a feature that the following parses:

test h = (+)92837492837h

Whitespace isn't that expensive anymore, and even cheaper on the black market ;-), so this "feature" does not seem worth preserving.

The moral: number literals should have to be terminated by one of the following: whitespace, punctuation (including parenthesis), operators etc, but not just by an alphabetic character. Maybe one can first parse a number literal as it was an identifier (i.e., allow leading numeric characters in identifiers), and then postprocess identifiers that start with a numeric character as number literals, giving parse errors if they do not make sense.

comment:4 Changed 4 years ago by vlopez

I fully agree with making it an error, and simplifying lexing of numeric literals.

However, there's at least one test case which would fail.¹ As for the Haskell report, there's no explicit rule about number literals being separated by whitespace.

I'd suggest giving a deprecated warning for 7.10. Then, for 7.12, the lexer can be switched to the new behaviour, and the language report updated accordingly.

  1. http://git.haskell.org/ghc.git/blob/c0fa383d9109800a4e46a81b418f1794030ba1bd:/testsuite/tests/parser/should_run/BinaryLiterals0.hs
Last edited 4 years ago by vlopez (previous) (diff)

comment:5 in reply to:  4 Changed 4 years ago by hvr

Replying to vlopez:

However, there's at least one test case which would fail.¹

...damn, I spent quite a bit of time to come up with that test-case to make sure I covered all code-paths and didn't deviate from the octal literal's lexing ;-)

comment:6 Changed 4 years ago by thoughtpolice

Milestone: 7.10.17.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

comment:7 Changed 3 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:8 Changed 3 years ago by thomie

Keywords: report-impact added

comment:9 Changed 2 years ago by thomie

Type of failure: None/UnknownIncorrect warning at compile-time

comment:10 Changed 2 years ago by thomie

Milestone: 8.0.1
Note: See TracTickets for help on using tickets.