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

Token from string #12

Merged
merged 5 commits into from
Jun 21, 2024
Merged

Conversation

JakeRoggenbuck
Copy link
Member

@JakeRoggenbuck JakeRoggenbuck commented Jun 18, 2024

closes #4

Copy link
Contributor

@adamhutchings adamhutchings left a comment

Choose a reason for hiding this comment

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

Just a minor thing and one thing that might be an error in the logic? Not sure

src/lexer/lex.c Outdated
}

// Does it start with a negative sign
if (contents[0] != '\0') {
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic here is redundant methinks (you only need the inner if), also why would we return an identifier? Wouldn't this return TT_IDENTIFIER for the string -= which is an operator?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, removed outer if

Copy link
Member Author

Choose a reason for hiding this comment

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

Will the lexer lex -= as a single token or will it do - and = separately?

src/lexer/lex.c Outdated
}

// Is it from "0123456789"?
if (c > 57 || c < 48 && c != 'u') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I might use the literal characters here instead of 57 and 48 so it's clearer

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup

Copy link
Contributor

@adamhutchings adamhutchings left a comment

Choose a reason for hiding this comment

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

Sorry to be pedantic

src/lexer/lex.c Outdated
}

// Does it start with a negative sign
if (contents[0] == '-') {
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know, the only tokens that can start with negative signs are -, -- and -=.

Copy link
Contributor

Choose a reason for hiding this comment

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

Am I forgetting something? Because I don't think those are allowed in identifiers in the first place

src/lexer/lex.c Outdated
}

// Count negative signs
if (c == '-') {
Copy link
Contributor

Choose a reason for hiding this comment

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

In C, the expression -2 is literally counted as - and then 2, as two separate tokens. In the exact same way as you can say int positive = 2, negative = -positive; or whatever.

@JakeRoggenbuck JakeRoggenbuck merged commit 6a4a851 into jabacat:main Jun 21, 2024
1 check passed
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.

Create a function to assign a type to a token based on its content
2 participants