Fix issue 19070 - some octal literals are actually allowed

[schveiguy]

No description provided.

[dlang-bot]

Thanks for your pull request, @schveiguy!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	19070	trivial	Octal literals `01` through `07` allowed, but not in the grammar

[schveiguy]

Note, this doesn't change any code, so targeting master/stable is irrelevant. Verify the result in the doc generator preview before merging.

[CyberShadow]

A funny related observation: leading zeroes are allowed for float literals, even when the integer part otherwise could be interpreted in octal with a different value:

auto i = 0011;   // Not allowed
auto j = 0011.0; // Allowed, decimal 11 (not 9)

[CyberShadow]

Actually this is still missing something:

int i = 0_0_1; // compiles, but invalid according to grammar

[CyberShadow]

Actually, hold on. There's multiple issues here.

First, this isn't quite right because it modifies DecimalInteger, which is used e.g. in DecimalFloat. However, looking at it closer, DecimalFloat isn't right either.

According to the grammar, .0001 is incorrect (because DecimalInteger, as used in DecimalFloat, does not allow leading zeroes). This pull request does fix it, but, it does not fix e.g. .0009.

So, as far as this change goes, I think it needs to be added to Integer or IntegerLiteral directly, and not DecimalInteger.

[schveiguy]

Will take a look at this later in the weekend. Can't today. Not surprised I messed it up! I thought DecimalInteger was a sibling of HexadecimalInteger.

[CyberShadow]

An instance in DMD's test suite:

https://github.com/dlang/dmd/blob/master/test/runnable/sdtor.d#L1062

[schveiguy]

I think the dependency on DecimalInteger is not correct. I'm going to rework a lot of this...

Which means this is no longer trivial, and needs a lot more attention to make sure I get it right!

[schveiguy]

OK, take a look now, let me know if you can find cases that aren't covered.

The DecimalFloat thing was a complete mess, and there were inconsistent references to other grammar elements. I can't think of a reason why, and my testing showed that the proposed grammar is correct (as far as underscores). I also discovered the hex float exponent wasn't hex, though the compiler allows it!

[schveiguy]

I also discovered the hex float exponent wasn't hex, though the compiler allows it!

Oh wow, I totally misunderstood this LOL. I did 0x1e__a and obviously, that e is not an exponent, but just part of a hex integer! Reverting that now.

[schveiguy]

ping @CyberShadow when you get a chance.

[CyberShadow]

Yep, definitely will get to this - sorry, this just caused a bit of yak shaving with me trying to approach this "properly".

[schveiguy]

ping!

[schveiguy]

ping again ;)

[schveiguy]

@CyberShadow would be nice to get this merged if you have time to review.

[CyberShadow]

Yes, sorry. It's just that the project which would allow me to properly review this is not at the top of my stack at the moment.

I understand that by your regular comments there is some pressure to merge this regardless, sooner rather than later?

[schveiguy]

No pressure, sorry. I just know that this is something that if I don't pay attention to it, will just fall off the end of my plate, and I'll forget, and then 5 years later it's still there. I get a tremendous amount of noise from github for D projects, and I can see this being missed if it were me.

I can stop pinging.

[schveiguy]

Almost a year since I asked about this, any news?

[gdamore]

FYI, I filed a DIP that would make these octal literals illegal.

IMO, I would rather do that, then add this stupid syntax -- rationale is in that PR:

dlang/DIPs#235

[schveiguy]

I'm ok with closing this and removing the feature. It's been a long time since I looked at this, I feel like I had some good changes in here, regardless of the octal digits thing, but I don't really care too much about refiling this.

[Lyratelle]

I think octal literals are forbidden long enough that we should now simply remove the rule that decimal literals may not start with 0.
0012 should simply mean 12 (0x0C). That would also ease the parsing of floatingpoint literals.
(BTW floatingpoint literals starting with dot should be deprecated altogether)

[ntrel]

0012 should simply mean 12 (0x0C)

D code should not change the meaning of valid C code without a good enough reason. Otherwise porting C code to D may cause latent bugs. It needs to remain an error.

[PetarKirov]

Otherwise porting C code to D may cause latent bugs.

While for most of D's history, this was a major design goal, one could argue that with ImportC, that's no longer the case. Perhaps there could be a switch say -transition=c-to-d, where the compiler would parse a given file twice - once with the ImportC parser and once with the normal D parser and output a diagnostic message for each language construct where D has different syntax or semantics based on the AST diff.