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

Give error when trying to extract a member of a constant vector/rot #64

Merged
merged 1 commit into from
Dec 3, 2017

Conversation

Sei-Lisa
Copy link

e.g. ZERO_VECTOR.x

Introduced in 24127a6 when constants were added to builtins.txt.

Fixes #63.

The error generated is "Invalid member .". Perhaps a syntax error would be better? llGetPos().x gives "syntax error, unexpected PERIOD". That error message would bear improvement, though.

But "Invalid member" is intended to report cases like e.g. "vec.w" so maybe it's worth reporting this as a syntax error instead.

e.g. ZERO_VECTOR.x

Introduced in 24127a6 when constants were added to builtins.txt.

Fixes #63.
@Sei-Lisa
Copy link
Author

Force-pushed a minor, OCD-like style fix.

@Sei-Lisa
Copy link
Author

Maybe an alternative error message could be: "syntax error, built-in constants don't allow member access"

The patch would look like this:

@@ -638,6 +638,9 @@ void LLScriptFunctionExpression::determine_type() {
 void LLScriptLValueExpression::determine_type() {
    LLScriptIdentifier *id = (LLScriptIdentifier *) get_child(0);
    id->resolve_symbol( SYM_VARIABLE );
+   if ( id->get_symbol() != NULL && id->get_symbol()->get_sub_type() == SYM_BUILTIN && id->get_member() != NULL ) {
+       ERROR( HERE, E_SYNTAX_ERROR, "syntax error: built-in constants don't allow member access" );
+   }
    type = id->get_type();
 }
 

and of course the new test case should be adjusted appropriately.

@Makopo Makopo merged commit cf881f4 into Makopo:master Dec 3, 2017
@Sei-Lisa Sei-Lisa deleted the sei-fix-const-vec-rot-elems branch December 3, 2017 16:12
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.

2 participants