-
-
Notifications
You must be signed in to change notification settings - Fork 369
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
Fix issue 19070 - some octal literals are actually allowed #3066
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request, @schveiguy! Bugzilla references
|
Note, this doesn't change any code, so targeting master/stable is irrelevant. Verify the result in the doc generator preview before merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
Actually this is still missing something: int i = 0_0_1; // compiles, but invalid according to grammar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
.
Will take a look at this later in the weekend. Can't today. Not surprised I messed it up! I thought |
An instance in DMD's test suite: https://github.com/dlang/dmd/blob/master/test/runnable/sdtor.d#L1062 |
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! |
da5c565
to
ab0d381
Compare
OK, take a look now, let me know if you can find cases that aren't covered. The |
Oh wow, I totally misunderstood this LOL. I did |
up some other numeric grammar issues.
ab0d381
to
5add5aa
Compare
ping @CyberShadow when you get a chance. |
Yep, definitely will get to this - sorry, this just caused a bit of yak shaving with me trying to approach this "properly". |
ping! |
ping again ;) |
@CyberShadow would be nice to get this merged if you have time to review. |
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? |
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. |
Almost a year since I asked about this, any news? |
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: |
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. |
I think octal literals are forbidden long enough that we should now simply remove the rule that decimal literals may not start with 0. |
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. |
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 |
No description provided.