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

Improve length-calculation of expressions containing labels #107

Open
Wolvereness opened this issue Sep 8, 2021 · 7 comments
Open

Improve length-calculation of expressions containing labels #107

Wolvereness opened this issue Sep 8, 2021 · 7 comments
Labels
enhancement New feature or request nolol Issues only regarding NOLOL

Comments

@Wolvereness
Copy link

Describe the bug

This line is too long (>70 characters) to be converted to yolol, even after optimization

To Reproduce

t = 0
f = 1
mode_check>
	:flight = f and :mode; :turret = t and :mode; GOTO mode_check*:mode+mode_swap*(not :mode)

mode_swap>
	f = not f
	t = not t
	:FcuRotationalPitch = 0
	:FcuRotationalYaw = 0
	:FcuRotationalRoll = 0
	:ModeColor = t + 3 * f
	:Mode = 1
	goto mode_check

Expected behavior

a=0 b=1
:flight=b and :mode :turret=a and :mode goto2*:mode+3*(not :mode)
b=not b a=not a :FcuRotationalPitch=0 :FcuRotationalYaw=0
:FcuRotationalRoll=0 :ModeColor=a+3*b :Mode=1 goto2

Platform:

  • VSCode: 1.60.0
  • vscode-yolol v0.1.9

Additional context
The code in question would preferably be like this:

t = 0
f = 1
mode_check>
	:flight = f and :mode
	:turret = t and :mode
	GOTO mode_check*:mode+mode_swap*(not :mode)

mode_swap>
	f = not f
	t = not t
	:FcuRotationalPitch = 0
	:FcuRotationalYaw = 0
	:FcuRotationalRoll = 0
	:ModeColor = t + 3 * f
	:Mode = 1
	goto mode_check

and compile to

a=0 b=1
:flight=b and :mode :turret=a and :mode goto2*:mode+3*(not :mode)
b=not b a=not a :FcuRotationalPitch=0 :FcuRotationalYaw=0
:FcuRotationalRoll=0 :ModeColor=a+3*b :Mode=1 goto2

If I replace mode_check and mode_swap with 2 and 3 respectively, it works. An alternative solution I had in mind would necessitate a different feature (see #108).

@Wolvereness Wolvereness added the bug Something isn't working label Sep 8, 2021
@dbaumgarten
Copy link
Owner

Thats not really a bug, it's more like a missing feature.
The compiler can not reliably determine the length of a "goto " until the very end of the compilation, when all line-positions are fixed and the values for the labels are known.

But the compiler needs to know a maximum length for that goto-statement, to figure out if lines can be merged (which happens way before the actual line-numbers are known). So currently it just assumes the goto will stay as long as it originally is.

I know this is bad, because it could result in errorneous "line-too-long" errors, but I haven't really found a proper solution for this.

Solving this 100% correctly would require trial, error and backtracking during compilation, which would be en enormous amount of work and also make compile-times longer.

@Wolvereness
Copy link
Author

Perhaps as a stopgap use a change of assumption to reduce variables and labels down to 2 characters? That would help, but there's still an edge case of 1 character.

@dbaumgarten
Copy link
Owner

That would probably be an improvement, and also relatively simple to do.

But that doesnt just leave the 1-character edge-case: Any expression that can be pre-computed at compile time (x=1+1 -> x=2), will be precomputed. Depending on what exact value the jump-label in the expression has, the resulting expression could be much shorter.

foo>
goto foo-10

Could result in things like "goto -10", "goto 0", "goto 10", while the heuristic would assume that it's gonne be "goto XX-20", which is quite a bit longer then the possible actual results.

@dbaumgarten
Copy link
Owner

The next best thing would be to insert any value from 1 to 20 for every jump-label in the expression, optimize the resulting expression and then use the length of the largest of the outcomes as an estimate.

@Wolvereness
Copy link
Author

Wolvereness commented Sep 8, 2021

Another consideration, that would address this particular issue: throw the error later in the process by assuming the line (using semicolons) would fit, and only when the final result fails does the error get thrown. That would work 100% of cases where the developer is smarter than the compiler trying to do what I wanted to do. This particular example has a label before (no combining upward), a label after (no combining downward), and semicolons (no breaking apart).

@dbaumgarten
Copy link
Owner

That might be a possibility. It doesn't solve the underlying issue, but would at least work in some specific cases.

@dbaumgarten
Copy link
Owner

Perhaps as a stopgap use a change of assumption to reduce variables and labels down to 2 characters? That would help, but there's still an edge case of 1 character.

That's what I have done now. Every jump-label will now be treated as having a lenght of 2.
That's still not perfect, but it's an improvement and I am currently lacking the time for a perfect solution.

I will rename this issue and keep it open for now, to serve as a todo.

@dbaumgarten dbaumgarten changed the title Line length optimization failure Improve length-calculation of expressions containing labels Sep 19, 2021
@dbaumgarten dbaumgarten added enhancement New feature or request nolol Issues only regarding NOLOL and removed bug Something isn't working labels Sep 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request nolol Issues only regarding NOLOL
Projects
None yet
Development

No branches or pull requests

2 participants