[m-rev.] for review: --warn-include-and-non-include

[Julien Fischer]

On Sat, 26 Jul 2025 at 05:16, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> Add a new option, --warn-include-non-include.
>
> When enabled, the compiler emits a warning for modules that contain
> both include_module declarations, and other constructs. In other words,
> it warns about modules that include submodules but are not "packages".

Is that restricted to exported submodules or does it apply all submodules, both
exported and private?  It is not uncommon for non-package modules to delegate
much of their implementation to private submodules.

...

> diff --git a/tests/warnings/help_text.err_exp b/tests/warnings/help_text.err_exp
> index a8e59d84d..ec39c52ed 100644
> --- a/tests/warnings/help_text.err_exp
> +++ b/tests/warnings/help_text.err_exp
> @@ -1070,6 +1070,23 @@ Warning options
>
>    Warnings about programming style
>
> +    Warnings about style issues with modules
> +
> +    --warn-include-and-non-include
> +        Warn about modules that contain both `include_module' declarations and
> +        other kinds of entities, such as types or predicates.
> +
> +        When a module contains both `include_module' declarations and other
> +        code, changes to that code will often cause the recompilation of all
> +        the included submodules. This is because those submodules have access
> +        to the entities declared in their parent module, so if the set of those
> +        entities changes in any way, the submodules must be checked to see
> +        whether they relied on some entity in the parent module that is not
> +        there anymore.
> +
> +        Modules that contain `include_module' declarations and nothing else,
> +        which are often called `packages', do not have this problem.

That's one form of "package". The other would be a module that just consists
of import_module declarations (i.e. to group a bunch of modules together into
a library).

> +
>      Warnings about style issues with predicates
>
>      --warn-dead-predicates

I'm not sure how useful this warning is going to be in practice. Thinking about
much of the code I work with I think there is a conflict between minimising
recompilations vs. additional complexity in an API due to users having to deal
with more submodules.

That said, the diff looks fine and I have no objections to it being committed.

Julien.