dlang.org icon indicating copy to clipboard operation
dlang.org copied to clipboard

Fix issue 19070 - some octal literals are actually allowed

Open schveiguy opened this issue 2 years ago • 21 comments

schveiguy avatar Jul 15 '21 19:07 schveiguy

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

dlang-bot avatar Jul 15 '21 19:07 dlang-bot

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

schveiguy avatar Jul 15 '21 19:07 schveiguy

Actually this is still missing something:

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

CyberShadow avatar Jul 16 '21 09:07 CyberShadow

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.

schveiguy avatar Jul 16 '21 12:07 schveiguy

An instance in DMD's test suite:

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

CyberShadow avatar Jul 18 '21 03:07 CyberShadow

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 avatar Jul 18 '21 15:07 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 avatar Jul 18 '21 16:07 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 avatar Jul 19 '21 15:07 schveiguy

ping @CyberShadow when you get a chance.

schveiguy avatar Jul 21 '21 15:07 schveiguy

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

CyberShadow avatar Jul 21 '21 17:07 CyberShadow

ping!

schveiguy avatar Aug 16 '21 15:08 schveiguy

ping again ;)

schveiguy avatar Sep 26 '21 15:09 schveiguy

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

schveiguy avatar Oct 13 '21 14:10 schveiguy

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?

CyberShadow avatar Oct 13 '21 14:10 CyberShadow

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 avatar Oct 14 '21 00:10 schveiguy

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

schveiguy avatar Sep 14 '22 15:09 schveiguy

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:

https://github.com/dlang/DIPs/pull/235

gdamore avatar Oct 27 '22 17:10 gdamore

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.

schveiguy avatar Oct 28 '22 00:10 schveiguy

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)

Lyratelle avatar Mar 31 '23 21:03 Lyratelle

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.

ntrel avatar Apr 02 '23 11:04 ntrel

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.

PetarKirov avatar Apr 03 '23 07:04 PetarKirov