[asdf-devel] Failures due to deferred warnings

[Faré]

Opn Thu, Feb 21, 2013 at 1:30 PM, Anton Vodonosov <avodonosov at yandex.ru> wrote:
> I want to say do not really support chaning the warinings handling.
> For two reasons:
>
> 1. I don't see how it can be introduced smoothly.
>     Faré, you did significant piece of work
>     by contacting all the maintainers,
>     and it will take more work to track their responses.
>     Somebody will not respond,
>     because the systems are not developed for several years,
>     somebeody will report the undefined variable if fixed,
>     but when we try we will find next undefined variable.
>     Warnings are not always trivial to fix.
>     It's not utf-8 where author can just set the :encoding attribute.
>     Fixing warning involves changing code,
>     analyzing how variables for functions are used,
>     is it possible to reorder definitions, and so on.
>     Also it may result in new bugs.
>
>     So I am affraid it will take half a year to push the fixes and it's work for you and for the maintainers.
>
>     Changing the warning rules without fixing the affected systems is not an option at all IMHO.
>     This CL codebase we inherit is a valueable asset, we should direct efforts to preservig it functioning,
>     but not breaking. If we want to direct evolution of CL into some more robust direction, it's better to
>     find a way without rejecting previous work.
>
Well, I agree that it might not be as smooth as we'd all like,
but on the other hand, as with any enforcement, it's the whole point of it
that it will catch errors that were not previously seen.

Pro checking deferred warnings by default:
P1- We don't go asking SBCL to remove new error checking features,
    or to downgrade errors to warnings, warnings to style-warnings,
    because these new checks find bugs in existing software.
    We welcome the checks and fix the bugs.
P2- It's trivial to disable if desired with
    (setf asdf:*warnings-file-type* nil)
    and get the old behavior back.
    For backwards compatibility, make it
    (defparameter asdf::*warnings-file-type* nil)
P3- If we don't enable it now, by induction, we won't enable it ever,
    because until we enforce it by default, there will always be
    someone somewhere who introduce new bugs.
    The overall cost of transitioning is not substantially reduced
    by postponing the transition.
P4- If some maintainers are unresponsive or unwilling to maintain anymore,
    it's time to either fork their work (if used) or drop it (if unused).
P5- As Attila Lendvai noted, users shouldn't be pulling changes to a library
    in isolation from changes to other libraries, but be using coherent
    snapshots of things that work together. And developers should be working
    on keeping their latest coherent with the latest of their dependencies.
    i.e. they should keep using the February snapshot of Quicklisp
    until it's time to upgrade to the March or April snapshot.

Con checking deferred warnings by default:
C1- If it was compiling cleanly (enough) and working before,
    why complain about it now? Preventing new bug is one thing,
    but can't we grandfather in old bugs we know don't affect us?
C2- Transition is smoother if old code doesn't have to be modified,
    only new code.
C3- Though the overall work is made somewhat larger by spreading it over time,
    the instant pain is less, and that matters, too.
C4- not having to either wait for the late maintainers
    or have some non-maintainer update in a fork allows us
    to make progress without any latency.
C5- Inasmuch as some software works for what it's used for,
    even if it otherwise has bugs, as long as these bugs are not exercised,
    it's better to let the software run than to reject it at compile-time.

So, how can we make transition smoother without sacrificing
checking for new code? I could
T1- have some magic transition date wired in asdf.lisp,
    and enable checking for any code modified after that date. Ouch.
T2- have a configurable list of systems that are grandfathered in,
    for which checking is disabled, there again
    with or without a date limit
    with or without some maximum applicable version number,
    with or without a non-empty default list wired in ASDF.
T3- introduce a new defsystem keyword :asdf-version, which specifies
    which asdf version a defsystem was written for, so we can guess
    what assumptions were put into the system; but then each and
    every user has to worry about putting a number not too recent
    yet not too old in there when he updates his software.
T4- implement some quicklisp variant that makes it easy to do
    non-maintainer upgrades for bug fixes to such issues.
T5- implement some Nix-style namespace management for Lisp systems
    and packages, so software can always be loaded with a coherent set
    of dependencies, including ASDF.

> 2. I do not really understand the change in warning handling. Of course, this point doesn't mean the
>    changes are wrong, and it's all in my hands to study the subject. But untill I get the understanding
>    I can't support this feature.
>
>    Faré, you said that the change introduces (with-compilation-unit (:override t) ... )
>    around compilation of every file, and the warnings reported by this with-compilation-unit
>    are saved and may result in compilation failure. In the previous ASDF there was only
>    one with-compilation-unit around whole ASDF system - this big with-compilation-unit
>    remains in new ASDF too.
>
>    Do I describe it right?
>
>    Why it is better? Do we catch more warnings or why?
>    What are the use-cases when new warnings behaviour make life better?
>
Yes, it is better because we catch actual warnings:
* functions used but never defined nor declared
* variables used before they were defined or declared

These are often mistyped function or variable names;
sometimes, it is missing functionality that is not implemented yet;
in any case, probable bugs.

>    And in general, warnings handling. I created a file warntest.lisp with this content:
>
>       (defun f () *my-var*)
>       (defparameter *my-var* 1)
>       (f)
>
>   On both CCL and SBCL, (compile-file "warntest.lisp") returns the following:
>
>       ;Compiler warnings for "c:/Users/anton/projects/cl-test-grid2/work/warntest.lisp" :
>       ;   In F: Undeclared free variable *MY-VAR*
>       #P"c:/Users/anton/projects/cl-test-grid2/work/warntest.wx64fsl"
>       T
>       T
>
>   Recall that CLHS calls the 3 values returned output-truename, warnings-p, failure-p.
>   So failure-p is T. Is this program non-conforming? Does it violate CLHS?
>
>   As far as I understand, this program is correct.
>
>   I creted also a wartest.asd and included wartest.lisp into this ASDF system.
>   Old ASDF performs load-op without error on this system. I am satisfied with
>   this behaviour. Will new ASDF fail this system?
>
The third value FAILURE-P is T, indicating a failure indeed,
which on SBCL would have caused us to fail the build
when checking the results of COMPILE-FILE,
as per *COMPILE-FILE-FAILURE-BEHAVIOUR*,
if the warning hadn't been deferred --
but it is deferred by WITH-COMPILATION-UNIT,
so an old ASDF will fail to fail the build
because it doesn't check the warnings.
A new ASDF will fail the build because it does check.

Forward reference to variables might even be non-conformant
(I remember an SBCL discussion about making it wholly illegal in SBCL,
but I don't remember what was the conclusion of the discussion).

>   I must say the warnings are very useful. When I work in SLIME they help
>   me all the time. But failing asdf:load-op on warnings by default is not good IMHO.
>   Warnigs are hints for developer.
>
>   If develper wants to be very clean, he should be able to enable for his code
>   a mode when warnings fail compilation.
>
That's an old discussion you should have had with Dan Barlow ten years ago.
You can change *COMPILE-FILE-FAILURE-BEHAVIOUR* if you want.
Downgrading it from :error to :warning would be a loss of functionality, IMHO.

>   But taking into account that different compilers produce different warnings,
>   this mode should not be enabled when system is delivered to other people.
>   Moreover, new versions of the same compiler can introduce new warnings.
>
>   I experienced such problem with big C programs, where build scripts enforce warning
>   free compilation. But since then gcc introduced many new warnings, so when
>   I wanted to rebuild the system myself there were hunderds of warnings.
>   It was unpleasant suprsie. I hoped to just "configure; make; make install",
>   but in result abandoned the program at all.
>
>   Therefore, I think it's better to use warning-clean mode only on the developer machine.
>
Maybe Quicklisp should bind *WARNINGS-FILE-TYPE* to NIL.
Or maybe ASDF should magically detect an old Quicklisp and do that.
(How do I magically detect an old Quicklisp?)

> Faré, I can help you with testing with warning, without warnings, as you want.
> But so far I don't see a win of the new warnings handling.
>
I agree that there's a transition issue to be handled.
The current situation is certainly unsatisfying.
I am not convinced what is the best solution going forward.

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
It is said an Eastern monarch once charged his wise men to invent him
a sentence to be ever in view, and which should be true and appropriate
in alltimes and situations. They presented him the words:
	"And this, too, shall pass away."
                — Abraham Lincoln