source: doc/proposals/nowarn.md @ df56e25

Making CFA compilation free of warnings
=======================================

Rationale
---------

A CS 343 student programming in CFA should be able to run our compiler with -Wall -Wextra, as the course standards mandate for uC++.  When doing so, all warnings seen should be caused by the student's choices.  At the close of 2024, about 70% of the CFA code written in the test suite produces warnings; anecdotally, about half of those are caused by CFA language features lowering to C code that does not satisfy strict warning rules.  So, CS 343 students are left swamped by warnings that they did not cause, and are powereless to prevent.  As a result they are more likely to ignore all the warnings and to be unable to learn from their real warnings about their real programming mistakes.

We should ensure that known causes of transpiler-induced C warnings are reduced or eliminated.  The body of this document gives guidance for CFA maintainers about how to do so.


Test Suite
----------

Test-suite compilation occurs in either strict or lax "wflags" mode.  Strict mode is -Wall -Wextra -Werror, so a test in strict mode cannot pass unless it is free of warnings.  Makefile.am owns a list of tests that opt for lax wflags (WFLGAS_OPT_LAX).  At the close of 2024, about 70% of the tests are on this list.

The `tests/nowarn` folder contains tests targeted at individual languge features that once caused warnings, but were since fixed.  Each such language feature is probably also tested in a general test.  Still, the targeted test is also valuable because, in the event of a regression, the targeted test has the outcome of the analysis that first had to occur on the general test.  This test also ensures the regression continues to be checked even if libcfa code changes to avoid using the problematic language feature.


Example fix
-----------

Upon this background, an example work cycle is now journaled.  The example change is committed along with the first draft of this proposal document.

The goal is to eliminate warnings seen in `array-collections/array-basic`.  Before the change, this test appears on WFLGAS_OPT_LAX.

To see the warning:
1. remove it from WFLGAS_OPT_LAX
2. run autogen.sh
3. rerun configure
4. remake cfa-cc/libcfa
5. run the test in test.py; expect failure, with the warning detail visible

Some warnings are only seen in release mode or on alternate architectures.  Useful command arguments to access some of these setups from plg2 include:
```
    ../cfa-cc/configure --with-target-hosts=host:debug,host:nodebug,x86:nodebug ...
    ./tests/test.py --arch x86 --debug no ...
```

Diagnosing the cause of the warning is not journaled.  In some cases, it is a programming mistake done directly in tests/foo.cfa; in such a case, the (part of the) fix is trivial.  The journaling follows a cause in cfa-cpp.

The cause of the `array-collections/array-basic` warnings is that there was unnecessary special handling for `zero_t`, which when showing up inside a thunk, caused the thunk to ignore one of its arguments.  With the cfa-cpp fix in place, the array test (still absent from WFLGAS_OPT_LAX) passes.

Consider adding a targeted `tests/nowarn` test, if there was nontrivial analysis involved in finding the cause of the warning.

The fix is a contribution toward the goal of reducing the WFLGAS_OPT_LAX list.  But work so far has only reduced it by one entry, even though it is probable that fixing a transpiler-induced warning cleans up warnings from several tests.  So far, such tests would be using WFLGAS_OPT_LAX unnecessarily, which a test.py run does not detect.

To identify the other tests now fixed:
1. Ensure that all tests pass.
2. Create a situation where all tests will be attempted with strict wflags.
   1. Comment out the value of WFLGAS_OPT_LAX, leaving the variable defined as empty.
   2. autogen, configure, make (as before)
3. Rerun all the tests, expecting many failures.  Each failure represents a test that cannot handle strict wflags.
   1. `./tests/test.py --all ... 2>&1 | tee test-status.txt`
   2. `grep 'PASSED  ' test-status.txt | sed -E 's/\s+([^ ]+).*$/\1/g' > test-passes.txt`
   3. `grep 'FAILED with' test-status.txt | sed -E 's/\s+([^ ]+).*$/\1/g' > test-fails.txt`
4. Trim the WFLGAS_OPT_LAX list, reconciling with the pass/fail status just obtained.
   1. Copy-paste just the WFLGAS_OPT_LAX items into `wflags-lax.txt`.  Keeping leading pounds and trailing backslashes is optional.
   2. Print a list of tests found in `wflags-lax.txt` and also in `test-passes.txt`.  These tests were originally on WFLGAS_OPT_LAX, but can now pass without being on it.
      - `sed -E 's/^([a-zA-Z])/(^|\\\\s|#)\1/g;s/\.\.\.//g;s/$/($|\\\\s|\\\\\\\\)/g' test-passes.txt | xargs -n 1 -I % echo "grep -E '%' wflags-lax.txt" | sh | sort`
   3. Note that in the running array-basic example, the resulting list was:
      - array-collections/array-sbscr-types (*)
      - ctrl-flow/loopctrl
      - userLiterals
      - vector_math/vec2_int
      - vector_math/vec2_float
      - vector_math/vec3_float
      - vector_math/vec4_float
      - zero_one
   4. Uncomment the official WFLGAS_OPT_LAX list.  Modify it to discard these tests.
   5. autogen, configure, make (as before)
5. Ensure that all tests pass, now under the reduced WFLGAS_OPT_LAX list.  Be deliberate about release mode and alternate architectures; i.e. that you aren't claiming to have fixed a test that always worked in x64-debug, but failed on a different configuration.  (Note that the overnight build will still catch that.)

(*) This test was found later, upon fixing a problem in these instructions, so the original commit does not show it as fixed, like it does for the others.