More warnings when compiling the compiler.

[alainfrisch]

This PR turns more warnings on for compiling the compiler. It already allowed to detect several bugs and would have avoided the one reported in #500.

The same treatment should probably be applied to stdlib and other parts of the code base.

[c-cube]

More warnings is good, imho. It would also be interesting to have more warnings become on by default, for instance 27.

[Drup]

@alainfrisch Thanks, this is very useful!

[gasche]

I think we should not merge such an invasive patch in trunk before the 4.03 branch is merged back into trunk after the release, because the risk of creating spurious merge conflicts is fairly large.

[bobot]

Why 4.03 branch can't be already merged back to trunk? And why can't it be done regularly in order to reduce the future conflicts?

[alainfrisch]

Interestingly, most of the warnings in stdlib and otherlibs that had to be fixed were related to the placement of "doc comments" (mostly -- but not only -- because of ";;" or lack of whitespace between declarations). I would have expected people working on codoc to have noticed that; does anyone know about the status of this project?

[alainfrisch]

As a side note, I also take the opportunity to enable -strict-formats in directory that I "clean up". The list of options to pass to stay "up to date" with best practice becomes longer and longer with each release (-strict-sequence, -strict-formats, -safe-strings). Perhaps one add one command-line flag to configure the compiler to behave as much as possible as a given version of OCaml (w.r.t. to such options and also default warning configuration), using by default the current version (i.e. the stricter one); and provide options to turn on/off each individual flag. This would give more visibility to new warnings and "strict modes", while making it trivial for released code not to be broken by a future stricter version.

[lpw25]

Interestingly, most of the warnings in stdlib and otherlibs that had to be fixed were related to the placement of "doc comments"

I very surprised as I distinctly remember turning that warning on already. Indeed if you look at #149 you can see that it includes a lot of fixes to doc comments in the stdlib.

does anyone know about the status of this project?

Work is ongoing. You can see an example of the current results here:

https://ocaml.janestreet.com/ocaml-core/latest/doc/

Should have something to release fairly soon, but it depends on how busy various people are.

[alainfrisch]

I very surprised as I distinctly remember turning that warning on already.

Could it be that the warning become stricter in subsequent commits?

Look for instance at:

cc86515#diff-1acf1effaa600a105c907affb358dd41

(fixes for hashtbl.mli) The problem here is caused by ambiguous comments between two declarations with no empty lines.

I can see that in #149, most of the fixes were about removing ";;" between the declaration and its doc (in post position), not really about inserting newlines to remove the ambiguity.

[lpw25]

Could it be that the warning become stricter in subsequent commits?

Yeah, I guess I must have turned the warning back off at some point, and then the comments stopped being updated to match the warning.

[trefis]

@bobot

Why 4.03 branch can't be already merged back to trunk? And why can't it be done regularly in order to reduce the future conflicts?

An alternative question would be: why branch so early? We could just prevent things we don't want in 4.03 to be merged before the release is done. This would avoid a lot of duplication (and of work, for people porting the patches from one branch to the other).

As for why we can't simply "git merge" 4.03 into trunk: there is, at least, one thing which diverges already and that you don't want to be merging: the travis script (which refers to different version of camlp4 on 4.03 and on trunk, cf. 16157ef).

[alainfrisch]

An alternative question would be: why branch so early?

We have been in "freeze" mode since early December. We cannot afford to remain for more than 3 months in a mode where nothing can land on trunk. There is a large overhead of maintaining unmerged PRs: a technical burden of having to synchronize them later; and a human problem that people working on something will lack interest/time/motivation to go back to it to do the tedious synchronization work N months later; plus the the fact that we increase the risk of conflicts between multiple PRs if they remain unmerged for too long.

[damiendoligez]

I agree with @alainfrisch: It's better to have a few conflicts between 4.03 and trunk, rather than N pull requests that conflict with trunk because they got out of date.

[bobot]

@trefis said:

As for why we can't simply "git merge" 4.03 into trunk: there is, at least, one thing which diverges already and that you don't want to be merging: the travis script (which refers to different version of camlp4 on 4.03 and on trunk, cf. 16157ef).

It is not just git merge but it is not very far, git merge 4.03 in trunk then git revert 16157ef in trunk. Next time you can do just git merge 4.03 the modification will not come back, and I even think that there will be no conflict on this travis line later. And your example indeed shows that it is better to do these merges early, just after the modifications in 4.03, otherwise we will forget that such reverts must be done.