[m-rev.] for review/discussion: change submodule visibility rules

[Peter Wang]

On Fri, 17 Jul 2026 14:22:29 +0200 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> 
> 
> On Thu, 16 Jul 2026 14:45:00 +1000, Peter Wang <novalazy at gmail.com> wrote:
> > I've attached the patch.
> 
> And I am attaching the review.
> 
> Zoltan.
> 

> 
> > If a submodule inherits visibility of M's exports in its implementation
> 
> If a descendant module of M ...
> 

M is the module being imported, so "descendant module of M" is not correct.
I've changed "submodule" to "module".

> > That only requires fairly simple changes to the compiler, and also seems to
> > me a decent language design. As Zoltan mentioned in bug #584, we may want to
> > require submodules to do all their own imports in the future anyway;
> > this rule change would be consistent with that direction.
> > 
> > doc/mercury_reference_manual.texi:
> >     Rename "Visibility rules" section to "Submodule visibility rules".
> > 
> >     Document new submodule visibility rules.
> > 
> >     Try to clarify existing visibility rules.
> > 
> > NEWS.md:
> >     Announce changes to submodule visibility rules.
> > 
> > compiler/prog_parse_tree.m:
> >     Add rwi1_ancestor_{int,imp}_{import,use} constructors.
> > 
> >     Add rwi3_direct_ancestor_{int,imp}_{import,use} constructors.
> > 
> >     (As it turns out, these additions might not be strictly necessary.)
> 
> For any of those constructors that are never generated, I would
> just comment them out in their type definition, with a comment.
> 

The new constructors are generated, but currently don't do anything
different from the existing non-ancestor constructors (due to
limitations in other code). I've kept the constructors.

> > --- a/compiler/module_qual.collect_mq_info.m
> > +++ b/compiler/module_qual.collect_mq_info.m
> > @@ -320,6 +332,7 @@ collect_mq_info_in_parse_tree_int1(ReadWhy1, ParseTreeInt1, !Info) :-
> >      % The implementation section of a .int1 file is abstract imported,
> >      % which means we have no permission for any item defined there.
> >      % We therefore do not need any ImpPermissions.
> > +    ImpPermissions = IntPermissions,
> 
> That comment seems to need updating.
> 

That change was a quick hack to satisfy
collect_mq_info_in_included_module_info, that I meant to revisit.
Based on the comment, I've added a variant of
collect_mq_info_in_included_module_info that only takes IntPermissions,
so ImpPermissions is not required.

> > --- a/doc/mercury_reference_manual.texi
> > +++ b/doc/mercury_reference_manual.texi
> 
> I have some proposals for how to improve the manual, but it would be
> simpler to make those changes myself after you commit.
> 
> > --- a/library/io.text_read.m
> > +++ b/library/io.text_read.m
> > @@ -18,11 +18,6 @@
> >  :- module io.text_read.
> >  :- interface.
> >  
> > -:- import_module bitmap.
> > -:- import_module list.
> > -
> > -%---------------------------------------------------------------------------%
> > -

I've undone those deletions. Those modules were already imported in the parent
module's interface section, along with other modules, but since bitmap and list
are specifically used in the io.text_read interface, I'll leave the imports there.

> >  :- pred read_word_2(io.text_input_stream::in, io.result(list(char))::out,
> >      io::di, io::uo) is det.
> >  
> > @@ -58,7 +53,6 @@
> >  :- implementation.
> >  
> >  :- import_module array.
> > -:- import_module char.
> >  :- import_module int.
> >  :- import_module int64.
> >  :- import_module io.primitives_read.
> 
> What happened to allow these deletions? Just for my interest.
> 

`char' (the type) occurs in the interface section of io.text_read so
io.text_read must either inherit the import of `char' (the module)
into the interface section from the parent module, or import `char'
in its own interface section. I've done the latter now.

> > diff --git a/library/string.builder.m b/library/string.builder.m
> > index 2c49fe84ef..5ea3c91d5f 100644
> > --- a/library/string.builder.m
> > +++ b/library/string.builder.m
> > @@ -2,7 +2,7 @@
> >  % vim: ft=mercury ts=4 sw=4 et
> >  %---------------------------------------------------------------------------%
> >  % Copyright (C) 2006-2007 The University of Melbourne.
> > -% Copyright (C) 2014-2015, 2018, 2022-2025 The Mercury team.
> > +% Copyright (C) 2014-2015, 2018, 2022-2026 The Mercury team.
> >  % This file is distributed under the terms specified in COPYING.LIB.
> >  %---------------------------------------------------------------------------%
> >  %
> > @@ -111,11 +111,6 @@
> >  
> >  :- implementation.
> >  
> > -:- import_module int.
> > -:- import_module list.
> > -
> > -%---------------------------------------------------------------------------%
> > -
> >  :- type state
> >      --->    state(list(string)).
> 
> And these?

Similar to above. I've moved the imports to the interface section
instead of deleting the imports.

> 
> I don't have the energy to have a look at the new test cases right now
> (will do so later), but the rest of the diff looks good, so I am
> happy for you to commit.

Thanks for the review. I've followed the rest of your suggestions.

I'll see if I can make the compiler accept code using the old visibility
rules, and warn about it.

Peter