GDShader allows commas in `for` conditions (middle part) incorrectly

[geekley]

Tested versions

• Reproducible in v4.2.2.stable.flathub [15073afe3]

System information

Godot v4.2.2.stable (15073afe3) - Freedesktop SDK 23.08 (Flatpak runtime) - X11 - Vulkan (Forward+) - integrated Intel(R) HD Graphics 5500 (BDW GT2) () - Intel(R) Core(TM) i5-5300U CPU @ 2.30GHz (4 Threads)

Issue description

GDShader is allowing commas in for middle part (condition), requiring every comma-separated part to be a boolean operator. Since GDShader (thankfully!) doesn't have a "comma operator" like in GLSL, this has to be a bug.

The compiler should either:

• Preferably, not allow a comma in the middle (condition) part of for, while still allowing it on 1st part (to declare multiple variables) and on 3rd part (to run multiple instructions, e.g. to do i++, j++). This is consistent with while, which does NOT allow commas.
• The worse option: allow , PROPERLY with comma operator semantics, so only the last comma-separated part has to be a boolean in a condition area. But I'm strongly against adding a comma operator that works everywhere, as it's a very confusing syntax. It's only actually useful in the 3rd part of for.

Additionally, I don't understand why the for condition part requires a boolean operator. It makes more sense to allow any expression returning a boolean, e.g. some bool function, some bool variable, literals true and false, etc.

Steps to reproduce

buggy.gdshader

shader_type canvas_item;

void fragment() {
  for (int i = 0, j = 0; 1 == 2, j <= 10; i++, j++)
    COLOR = vec4(float(i)/10.0, float(j)/10.0, 0, 1);
  // yellow texture shows the comma operator is being accepted on the `for` condition (2nd part) too
  // but the editor incorrectly expects every part of the comma to be a boolean operator expression
}

Minimal reproduction project (MRP)

N/A

[Chaosus]

I think this is not a bug, because middle expression commas are allowed on GLSL and C-language family. Only a last expression has been taken into account in such case + if we change it - this will cause a compatibility breakage.

Additionally, I don't understand why the for condition part requires a boolean operator. It makes more sense to allow any expression returning a boolean, e.g. some bool function, some bool variable, literals true and false, etc.

It makes sense, would you like to create a new issue about it?

[geekley]

@Chaosus It is still an issue because, even if you go the route of allowing comma operator semantics only in for, then like you said, only the last expression (i.e. z in a, b, ..., z) should be considered (the previous parts must run, but their return value must be ignored). The issue is that the compiler is requiring EVERY comma part (i.e. a, b, etc) to be a boolean. I call it a bug because it makes it seem as if the comma means something else, like e.g. an AND or OR operator (which do get passed boolean arguments).

You should only require the last comma part to be a boolean, the others could be anything because their return value is ignored. I think even a void function call is allowed? Actually no, just tested void in ShaderToy with error: for (int i=0; void_fn(), i < 1; i++);

',' : sequence operator is not allowed for void, arrays, or structs containing arrays

[geekley]

So, to clarify, this valid code is accepted: for (int i = 0, j = 0; 1 == 2, j <= 10; i++, j++) But this valid code isn't being accepted: for (int i = 0, j = 0; (j = f(j)), j <= 10; i++, j++)

Btw, if you go the route of allowing comma operator semantics even in for conditions, then I'd expect it to be also allowed at least in while and do while conditions for consistency (again, requiring only the last part to be a boolean). E.g.:

while (i = f(i), i < n) foo();
do foo(); while (i = f(i), i < n);