Skip to content
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

don't allow a token starting with an asterisk directly following .* #6858

Merged
merged 6 commits into from
Oct 30, 2020

Conversation

travv0
Copy link
Contributor

@travv0 travv0 commented Oct 28, 2020

closes #6823

I couldn't find where the tests are for the tokenizing so if one needs added if you could point me in the right direction that'd be great

@travv0 travv0 marked this pull request as draft October 28, 2020 22:13
@Vexu
Copy link
Member

Vexu commented Oct 28, 2020

You have to also change this in lib/std/zig/tokenizer.zig which has tests in it, you should also add a test in test/compile_error.zig.

@travv0
Copy link
Contributor Author

travv0 commented Oct 28, 2020

Sounds good, I noticed the zig tokenizer a few minutes ago and was gonna ask if I should mirror the changes there.

@travv0 travv0 marked this pull request as ready for review October 29, 2020 03:47
@@ -1001,6 +1000,17 @@ pub const Tokenizer = struct {
},
},

.period_asterisk => switch (c) {
'*' => {
result.id = .Invalid;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To get a proper error message you have to give this a new tag. You should be able to copy what I did with .Invalid_ampersands.

@travv0
Copy link
Contributor Author

travv0 commented Oct 29, 2020

So just to make sure I understand, you're returning a valid node since it actually is a complete token even though it's illegal for it to be directly followed by an asterisk? So it's more of a style error than an "I don't know how to continue to parse from here" error.

Edit: Never mind, answered in Discord.

@Vexu Vexu merged commit 80dd432 into ziglang:master Oct 30, 2020
@travv0 travv0 deleted the no-star-after-dot-star branch October 30, 2020 14:09
@@ -323,6 +326,7 @@ pub const Error = union(enum) {
pub const ExtraAllowZeroQualifier = SimpleError("Extra allowzero qualifier");
pub const DeclBetweenFields = SimpleError("Declarations are not allowed between container fields");
pub const InvalidAnd = SimpleError("`&&` is invalid. Note that `and` is boolean AND.");
pub const AsteriskAfterPointerDereference = SimpleError("`.*` can't be followed by `*`. Are you missing a space?");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 spaces before Are you missing a space?.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the user's missing space, silly!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code golf reveals shocking discovery: .* and ** don't require a space!
4 participants