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

Implement Symbol:isBinaryOp and Symbol:isIdentifier #2955

Merged
merged 9 commits into from Jun 16, 2017

Conversation

nhthn
Copy link
Contributor

@nhthn nhthn commented Jun 15, 2017

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.

Nathan Ho added 7 commits June 15, 2017 09:55
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.
@nhthn nhthn added the comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library" label Jun 15, 2017
@nhthn nhthn added this to the 3.9 milestone Jun 15, 2017
@nhthn nhthn force-pushed the topic/symbol-identifier-binaryop branch from 2facbf2 to 6b4d613 Compare June 15, 2017 18:30
Copy link
Member

@telephon telephon left a 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.

@nhthn
Copy link
Contributor Author

nhthn commented Jun 15, 2017

sorry, what?

@telephon
Copy link
Member

sorry, I misread. Never mind!

@@ -179,6 +179,82 @@ int prSymbolIsMetaClassName(struct VMGlobals *g, int numArgsPushed)
return errNone;
}

const char *binary_op_characters = "!@%&*-+=|<>?/";
Copy link
Contributor

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

Copy link
Contributor

@mossheim mossheim Jun 15, 2017

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.

Copy link
Contributor Author

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];
Copy link
Contributor

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".
@nhthn nhthn force-pushed the topic/symbol-identifier-binaryop branch from 05742f5 to 3e0e7a7 Compare June 15, 2017 22:32
Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

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

awesome thanks!

@telephon telephon merged commit a2bf1ef into supercollider:master Jun 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants