-
Notifications
You must be signed in to change notification settings - Fork 757
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
Implement Symbol:isBinaryOp and Symbol:isIdentifier #2955
Implement Symbol:isBinaryOp and Symbol:isIdentifier #2955
Conversation
This adds a list of characters that are valid in binary operators.
This commit implements Symbol:isBinaryOp via the primitive _Symbol_IsBinaryOp. For benchmarking purposes, this commit also includes Symbol:isBinaryOpLang, implemented natively.
This commit implements a method to check whether a symbol is a valid identifier (variable name) via the primitive _Symbol_IsIdentifier.
2facbf2
to
6b4d613
Compare
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.
are we syntactically sure that such binary ops will be single letter? I suppose this depends on the lexer/tokenizer.
sorry, what? |
sorry, I misread. Never mind! |
@@ -179,6 +179,82 @@ int prSymbolIsMetaClassName(struct VMGlobals *g, int numArgsPushed) | |||
return errNone; | |||
} | |||
|
|||
const char *binary_op_characters = "!@%&*-+=|<>?/"; |
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.
makes more sense as static
within prSymbol_IsBinaryOp
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.
Sorry, should have been clearer—you can put them inside the function prSymbol_IsBinaryOp
with the static
storage classifier, and that means they are treated as init-once storage within the function but with local scope.
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.
ok, fixed
SetTrue(a); | ||
// All other characters must be alphanumeric or '_'. | ||
for (int i = 1; i < length; i++) { | ||
char the_character = str[i]; |
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.
'c' would be much more idiomatic here
Make a string static and change "the_character" to "c".
05742f5
to
3e0e7a7
Compare
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.
awesome thanks!
implement #2952. we're going to be using these methods in SCDoc so why not make 'em faster with primitives. also, i need some experience writing primitives.
all documented, unit tested, and ready to go.