Replace goto statements

[galanCA]

Description

Replaces a goto statement for an inline function to exit sunLitFract function.

Author Progress Checklist:

• Open draft pull request
  • Make title clearly understandable in a standalone change log context
  • Assign yourself the issue
  • Add at least one label (enhancement, bug, or maintenance)
  • Link the issue(s) addressed by this PR (under "Development" in the sidebar menu)
• Make code changes (if you haven't already)
• Self-review of code
  • My code follows the style guidelines of this project
  • I have added comments to my code, particularly in hard-to-understand areas
  • I have only committed the necessary changes for this fix or feature
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have ensured that my fix is effective or that my feature works as intended by:
    • exercising the code changes in the test framework, and/or
    • manually verifying the changes (as explained in the the pull request description above)
  • My changes pass all local tests
  • My changes successfully passes CI checks
  • Add any unit test for proof and documentation.
  • Merge in main branch and address resulting conflicts and/or test failures.
• Move pull request out of draft mode and assign reviewers
• Iterate with reviewers until all changes are approved
  • Make changes in response to reviewer comments
  • Merge in main branch and address resulting conflicts and/or test failures.
  • Re-request review in GitHub

Reviewer Checklist:

• Read the pull request description
• Perform a code review on GitHub
• Confirm all CI checks pass and there are no build warnings
• Pull, build, and run automated tests locally
• Perform manual tests of the fix or feature locally
• Add any review comments, if applicable
• Submit review in GitHub as either
  • Request changes, or
  • Approve
• Iterate with author until all changes are approved

[galanCA]

Moving @chipbarnaby's comment here.
#318 (review)

[galanCA]

@chipbarnaby Would it be ok if I replace it for a function? Perhaps an inline function?

[tanaya-mankad]

What was the reason for changing just exit68 to a function, and not other gotos?

[galanCA]

Because exit68 was getting an error. I can't recall the error at the moment. I think it had something to do with skipping redefinitions of some variables.

[tanaya-mankad]

The error occurs because as far as the compiler is concerned, X3 could be used under the exit68 label; its scope allows that. If it were declared without initialization (i.e. float X3;) it would compile, because then it's not the compiler's job to ensure it has a valid value at any point. But when we initialize it in place (float X3 = FD * hor;) then the compiler must ensure it's initialized before any potential jump that might use it.
A number of solutions are possible. We can limit the scope of X3 in a couple of ways (curly braces around the whole block that uses it, redeclare/initialize it inside each scope that needs it, etc.). We could declare it and then assign it, but that usually leads to uninitialized-variable warnings. Or we could solve the problem the way you did, by using an inline function.
Just remember that we're not fond of simply choosing a solution that "makes the compiler shut up."

[galanCA]

Is limiting the scope the best out of all of those options?

[chipbarnaby]

Not inline! That would just duplicate the code at each call. You could use a lambda or non-inline function. (Or leave the goto. I really don't see what is wrong with it. We need a policy discussion -- absolutely always outlawing some approach does not make sense in my opinion.)

…

On Wed, Jun 15, 2022 at 5:23 PM Cesar Galan ***@***.***> wrote: @chipbarnaby <https://github.com/chipbarnaby> Would it be ok if I replace it for a function? Perhaps an inline function? — Reply to this email directly, view it on GitHub <#319 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACWGW7BKYKJCFQP4464BB6DVPJCU7ANCNFSM5Y4VJO7A> . You are receiving this because you were mentioned.Message ID: ***@***.***>