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

Change literal value of s_none symbol in PyrObject #2638

Merged
merged 3 commits into from
Feb 3, 2017
Merged

Change literal value of s_none symbol in PyrObject #2638

merged 3 commits into from
Feb 3, 2017

Conversation

mossheim
Copy link
Contributor

Fix for #2614.

s_none is needed in the current version of the class and primitive parser as a dummy symbol for "no superclass" or "no name", because other more serious dependency tests are performed using nullptr. However, the use of the symbol literal none caused the interpreter to crash if a method called "none" was added to a SC class, presumably because the parser would attempt to point the new method name to the existing dummy symbol and then run into problems later while dependencies were being constructed.

By changing the literal definition of s_none to __none I believe we solve this issue with minimal effort by keeping the symbol out of the method and primitive namespaces, while preserving the semantic value of the literal.

To test, I modified the BasicOpUGens.sc file to begin:

BasicOpUGen : UGen {
   	var <operator;

	none {
		"this works".postln;
	}
...
}

then built and tested sclang via command line:

*** Welcome to SuperCollider 3.9dev. *** For help type cmd-d.
sc3> a = BasicOpUGen.new
-> a BasicOpUGen
sc3> a.none
this works
-> a BasicOpUGen
sc3> a.perform('__none')
[method not understood error message]

I ran similar tests with none as a class method, classvar, instance var, and both classvar and instance var, all with similar positive results.

s_none = getsym("none");
s_none = getsym("__none");
/* Dummy symbol for null superclass or primitive. Prefixed with
* `__` to avoid collisions with possible method names.
Copy link
Member

Choose a reason for hiding this comment

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

I suppose that the double _-prefix (instead of a single one) is needed because single prefix _ is reserved for primitive calls? So maybe you could write

`__` to avoid collisions with possible method and primitive names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose that the double _-prefix (instead of a single one) is needed because single prefix _ is reserved for primitive calls?

Technically it's not necessary, since primitives begin with _[A-Z], but this way it's clear that it's part of neither namespace.

@nhthn nhthn added the comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library" label Jan 13, 2017
@nhthn nhthn added this to the 3.9 milestone Jan 13, 2017
@mossheim
Copy link
Contributor Author

mossheim commented Feb 2, 2017

Needed some slight resolution after I merged my other PyrObject.cpp edit. Waiting for travis build to complete for good measure, then I'll merge.

@mossheim mossheim merged commit 913ecd2 into supercollider:master Feb 3, 2017
@mossheim mossheim deleted the topic/allow-methods-named-none branch February 3, 2017 00:10
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.

3 participants