Option to treat warnings as errors when compiling and rendering a .scad file

[brantmerryman]

Currently if you include or use a file inside of a .scad document, and that file is not found, a warning is generated and the object still renders.

Often this means that a missing include file can cause holes to be the wrong size.

To do:
Add a compiler flag to treat warnings as errors. This should be something similar to the gcc options:
* -Werror treat all warnings as errors
* -Werror= specific error treat a specific warning as an error
* -Wno-error= specific error treat a specific warning as a warning even when -Werror is in effect
We would like to at least cover the case of a missing file.

Add a way to specify this inside of the GUI, possibly as a preferences item: "treat warnings as errors".

Nice to have, but might be too fancy Add a way to modify the current compiler flags from code - similar to #pragma.

[kintel]

Some additional ideas/comments:

• From the cmd-line, errors should cause a non-zero return value
• We could add the return value also to the test drivers, and add tests for the misc errors/warnings
• Handle errors and warnings as such (error = abort, warning = continue). This probably means that some of today's warnings should become errors.
• Add a compiler flag to ignore errors (or backwards compatibility or use-cases not covered here)
• I think it makes sense to fail on errors as default on the cmd-line (and set the corresponding GUI preferences flag as default).

If you need assistance in any way, send me and email or catch me on IRC (or any other IM you may be using)

[marcosscriven]

Just saw this issue in my email, and it's also something I thought about.

I definitely think this should be the other way around. I.e. It should be an 'error', and a user can, if they know the missing item isn't a big deal, choose to deal with such errors as 'warnings'. But as brantsears says, a simple typo can end up generating something totally different.

Sorry if this is a dumb question, but why would you want to make this a compile-time option? Shouldn't compile options be down to environmental choices, as supposed to behaviour? I wouldn't expect an OpenSCAD binary to behave differently down to how it was compiled.

[kintel]

"compile-time" in this context means compiling an openscad script. It's not technically compiling, but parsing and evaluation. The flags are just inspired by gcc.

[kintel]

From Triffid on the mailing list:

Greg Frost's extruders import STLs of his gears if they're present, for display purposes only. If they're absent, the warning is harmless. FIXME: Link to these models.

SO, we should have a way to treat it as an error, and have a way to get a mere warning if the import is only there for display purposes and isn't necessary for the actual output

perhaps %use or %import could trigger warning-only mode or something?

[donbright]

i dont have anything to say here except that i liked your comment against confusing 'sugar' punctuation. something more pythonic might be something like

  include("blah.scad",ignore_error=true) 

[kintel]

From drewm1980:

If you call a function that does not exist, it should be an ERROR not a WARNING:

norm([1,1,1]);
WARNING: Ignoring unknown module 'norm'.

Not noticing this (and just looking at the rendered output) cost me several hours of development time. ("norm" is not in the version of openscad in Ubuntu 12.04)

Also, doing ANYTHING with an "undef" value should cause an ERROR. For example, translate(undef) should NOT silently be a translation by zero!

If there are a lot of bugs this where files with blatant syntax errors can still compile and render, a flag to turn all warnings into errors akin to gcc's -Wall would be very welcome.

[GilesBathgate]

With regard to the above comment:

I agree completely with the first statement. Any ambiguity where the compiler can make a sensible guess should be a warning. When the compiler cannot understand what you mean because the module you are referring to is not available, then this is definitely an error. I don't agree that translate(undef) should be an error. There could be a case where you want to take advantage of that. e.g.

module my_part(a,b,optional_translate=undef)
{
 translate(optional_translate) cylinder(a,b);
}

[MichaelAtOz]

+1 Module call to non-existent module = ERROR
No so sure re undef. But undef should cause a warning always.
Also particularly the current silent errors where you do something stupid and it produces something but just ignores things you no doubt had a typo in, needs a warning. ( I'll have to remember to collect examples if you don't know what I mean)

[5solids]

By the way, does anybody know of any way to cause an error that stops the machinery?

For the time being, I could live with defining my own module and simply calling that in case I'am not happy with a configuration parameter or similar. For instance:

module exit(rc) {
echo(str("RC: ",rc));
// something that stop the compiler
}

if (foo == 23) {
echo("ERROR: invalid foo value");
exit(1);
}

Another alternative could be to have an assert() function.

Regarding checking if an include worked, it might be a work around to add a dedicated variable in the include file and to check its value after the import. But then again it would be great if we would have a way to programmatically stop the compile.

[kintel]

For assert(), see #381

[jordanbrown0]

+1. It's quite annoying to make a mistake, have F5 immediately point out the mistake, but then have to wait for a known-bad re-render.

[kintel]

@MichaelPFrey This could be a great potential add-on to all the great warning work

[MichaelPFrey]

With regard to the above comment:

I agree completely with the first statement. Any ambiguity where the compiler can make a sensible guess should be a warning. When the compiler cannot understand what you mean because the module you are referring to is not available, then this is definitely an error. I don't agree that translate(undef) should be an error. There could be a case where you want to take advantage of that. e.g.

module my_part(a,b,optional_translate=undef)
{
 translate(optional_translate) cylinder(a,b);
}

there are various ways to avoid the translate(undef) in this example. Making the module proper:

module my_part(a,b,optional_translate=[0,0])
{
 translate(optional_translate) cylinder(a,b);
}

If you really want to get messy with undef for some other reason:

module my_part(a,b,optional_translate=undef)
{
 translate(optional_translate==undef ? [0,0] : optional_translate) cylinder(a,b);
}

module my_part(a,b,optional_translate=undef)
{
 translate(is_undef(optional_translate) ? [0,0] : optional_translate) cylinder(a,b);
}

[MichaelPFrey]

allowing a scad file to overwrite hardwarning "on the fly" would have to be at two or three parter, as the implementation has to take into account the different phases of parsing, evaluating and rendering the script.

Specifying which warnings are effected would require labeling all warnings first.

From Triffid on the mailing list:

Greg Frost's extruders import STLs of his gears if they're present, for display purposes only. If they're absent, the warning is harmless. FIXME: Link to these models.

SO, we should have a way to treat it as an error, and have a way to get a mere warning if the import is only there for display purposes and isn't necessary for the actual output

perhaps %use or %import could trigger warning-only mode or something?

In such a case, we should consider addding a flag to the import module. I prefer to tweak the language, so that people can state their intention in a clean way in the script where things happen (see the undef example above), instead of giving them options to mess with the error/warning reporting/handeling in general.

[t-paul]

I'm not fully convinced this belongs into the script. If there's warnings someone is concerned about in their own code - fix it. Missing files (like given as example in the OP) are more an environment issue so the option makes more sense in the UI / compilation call - similar to the -Wall group of options of gcc.

One of the main things to consider is that the number of warnings can change over time without code change, like due to new OpenSCAD versions being used. Breaking libraries that some time ago tried to be warning clean (which is good) but flagged that as requirement (which is bad) is not helping.

[nophead]

I agree. I don't think adding to the language is a good idea at all as you would have all the scope issues to bear in mind.

A command line option and a preference setting should be fine and is how all the other things that effect the language are currently set.

[ringerc]

I agree that it'd make a lot of sense to be able to make warnings fatal, with the option of it being global or scoped to a given include file. Perhaps extend the use directive to allow setting of file-scoped variables like `$

As I suggested in a comment on #381 it'd be great to be able to use the warnings functionality explicitly in modules too, by adding a warning(...) function.

[t-paul]

Option exists now.

[dclaze]

@t-paul how do we use this new option? Could you point to some documentation on what is needed to enable this "treat warnings as errors" behavior?

[t-paul]

Edit -> Preferences -> Advanced -> OpenSCAD Language Features -> ☑️ Stop on the first warning