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

Range Warnings begin<end #3149

Merged
merged 6 commits into from
Dec 11, 2019
Merged

Range Warnings begin<end #3149

merged 6 commits into from
Dec 11, 2019

Conversation

MichaelPFrey
Copy link
Member

@MichaelPFrey MichaelPFrey commented Nov 30, 2019

based on http://forum.openscad.org/Infamous-warnings-td28054.html

  • remove the now redundant code in value.cc
  • consider generalizing warning
    • (just focusing on literals for now)
    • Warn for
      • for(i=[10:1]) -> swap, deprecated
      • for(i=[1:-1:10]) -> warning
      • for(i=[1:0:10]) -> bad range warning (not very specific, but "fair enough" with the line number)
    • correct (do not warn)
      • for(i=[0:10]) -> fine
      • for(i=[10:-1:1]) -> fine
    • not addressed with this PR
      • expression containing variables like for(i=[0:a]) for(i=[a:0]) for(i=[0:a:10]) for(i=[a:1:10]) for(i=[0:1:a])
      • non-numbers like
        • for(r=[1:true:5]) echo(r);
        • for(r=[1:"test":5]) echo(r);
        • for(r=["a":"z"]) echo(r);
  • test cases
    • (check list from above)
    • manually review changed test cases
  • move the warning to a NOINLINE function to minimize memory footprint?
  • rephrase warnings for consistency

@nophead
Copy link
Member

nophead commented Nov 30, 2019

Isn't for(i=[10:-1:1]) the legal way to iterate backwards? I thought what was deprecated was automatically changing the implicit increment to -1 when the range was backwards. Not banning backwards ranges altogether.

@MichaelPFrey
Copy link
Member Author

concate-tests has something interesting:
Bildschirmfoto von 2019-11-30 13-59-57-1

echo(concat([3, 4, true, 6], [4:1:3]));

for-tests has something interesting:

// Illegal step value
for(r=[1:true:5]) translate([r*10-60,50,0]) cylinder(r=r);

some more examples:
Bildschirmfoto von 2019-11-30 14-15-41-1

@MichaelPFrey
Copy link
Member Author

MichaelPFrey commented Nov 30, 2019

Isn't for(i=[10:-1:1]) the legal way to iterate backwards?

Yes it is - I should separate out "should not create warnings" from "should create warnings" and "not my concern right now" in my to do list to be more clear.

@kintel kintel merged commit f7b5ce5 into openscad:master Dec 11, 2019
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.

3 participants