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

Parse error in multiline subscription expression #35417

Closed
Tracked by #40488
Scony opened this issue Jan 21, 2020 · 9 comments
Closed
Tracked by #40488

Parse error in multiline subscription expression #35417

Scony opened this issue Jan 21, 2020 · 9 comments

Comments

@Scony
Copy link
Contributor

Scony commented Jan 21, 2020

Godot version:
master @ d11d7df

OS/device including version:
Linux

Issue description:
Godot raises parse error on the code below:

class X:
	func foo(p):
		var x = p[
			1
		]
Godot Engine v3.2.rc.custom_build.d11d7dfe3 - https://godotengine.org
 
SCRIPT ERROR: GDScript::reload: Parse Error: Error parsing expression, misplaced: '\n'
   At: res://tests/potential-godot-bugs/multiline-subscription-expression.gd:4.
ERROR: reload: Method/Function Failed, returning: ERR_PARSE_ERROR
   At: modules/gdscript/gdscript.cpp:576.
@bojidar-bg
Copy link
Contributor

Adding parenthesis++ and parenthesis-- around this line should resolve the issue:

Node *subexpr = _parse_expression(op, p_static, p_allow_assign, p_parsing_constant);
if (!subexpr) {

Note that the following code would continue erroring, but it should be resolved with #35415:

var x = (z
    [p])

@RandomShaper
Copy link
Member

I don't think that should be changed. The language is just designed after Python in that regard: to require parentheses for allowing line breaks in expressions).

(See #208 (comment) for context.)

@lakshay-angrish
Copy link
Contributor

@RandomShaper
Shouldn't the newline after [ be ignored by the following code?

} else if (tokenizer->get_token() == GDScriptTokenizer::TK_BRACKET_OPEN) {
// array
tokenizer->advance();
ArrayNode *arr = alloc_node<ArrayNode>();
bool expecting_comma = false;
while (true) {
if (tokenizer->get_token() == GDScriptTokenizer::TK_EOF) {
_set_error("Unterminated array");
return NULL;
} else if (tokenizer->get_token() == GDScriptTokenizer::TK_BRACKET_CLOSE) {
tokenizer->advance();
break;
} else if (tokenizer->get_token() == GDScriptTokenizer::TK_NEWLINE) {
tokenizer->advance(); //ignore newline
} else if (tokenizer->get_token() == GDScriptTokenizer::TK_COMMA) {
if (!expecting_comma) {
_set_error("expression or ']' expected");
return NULL;
}
expecting_comma = false;
tokenizer->advance(); //ignore newline
} else {
//parse expression
if (expecting_comma) {
_set_error("',' or ']' expected");
return NULL;
}
Node *n = _parse_expression(arr, p_static, p_allow_assign, p_parsing_constant);
if (!n)
return NULL;
arr->elements.push_back(n);
expecting_comma = true;
}
}
expr = arr;

@RandomShaper
Copy link
Member

I think that's for the context when an array is declared, to allow one item per line:

var a = [
    1,
    2,
    3,
]

What is happening in the example code is indexing, which is parsed here:

} else if (tokenizer->get_token() == GDScriptTokenizer::TK_BRACKET_OPEN) {

@bojidar-bg
Copy link
Contributor

I don't think that should be changed. The language is just designed after Python in that regard: to require parentheses for allowing line breaks in expressions).

[ and ] are technically parenthesis.
I mean, the following two are identical:

p[(x + y)]
p[x + y]

So it makes sense that the following two are:

p[(
  x + y
)]

p[
  x + y
]

@RandomShaper
Copy link
Member

I wasn't too convinced about your point since I thought the rule was intended for actual parentheses, rather than expression "enclosing".

I was wrong.

Moreover, regardless any other consideration, since Python allows it and the intent is that GDScript imitates its syntax, it should be allowed.

@vignesh-j-shetty
Copy link
Contributor

This issue doesn't seem to be there in master branch.

@Calinou Calinou added this to the 3.2 milestone Feb 20, 2021
@akien-mga akien-mga modified the milestones: 3.2, 3.3 Mar 17, 2021
@akien-mga akien-mga modified the milestones: 3.3, 3.5 Oct 25, 2021
@akien-mga
Copy link
Member

akien-mga commented Oct 25, 2021

There was a proposed fix in #35597 which couldn't be reviewed/merged before the author deleted their fork. Could likely be salvaged as it seems to be a valid fix, the same was done for preload() in #52521 as advised by @vnen.

Edit: I'll do it.

akien-mga added a commit to akien-mga/godot that referenced this issue Oct 25, 2021
sairam4123 pushed a commit to sairam4123/godot that referenced this issue Nov 10, 2021
akien-mga added a commit to akien-mga/godot that referenced this issue Nov 15, 2021
lekoder pushed a commit to KoderaSoftwareUnlimited/godot that referenced this issue Dec 18, 2021
@dalexeev
Copy link
Member

Fixed by #54227.

@vnen vnen closed this as completed Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants