[m-rev.] for review: fix bug #230
juliensf at csse.unimelb.edu.au
Tue Nov 22 17:53:01 AEDT 2011
Thanks for looking at this so promptly.
On Tue, 22 Nov 2011, Peter Wang wrote:
> Julien, I assume this fix would be too intrusive for 11.07, so you
> should go ahead with disabling the direct arg functor optimisation there.
Done - I will have a look at merging the changes on to the 11.07 branch
*after* the release; we might re-enable direct arg functors in 11.07.
> Branches: main
> Fix bug #230. When compiling a sub-module we could incorrectly use the
> direct argument functor representation for a d.u. functor when:
> 1. the outer type is defined and exported from an ancestor module, and
> 2. the functor argument type is defined in the same ancestor module,
> but NOT exported.
That's not clear; the functor argument type is (abstract) exported; it's
the definition of the functor argument type that is not exported.
> :- module ancestor.
> :- interface.
> :- type outer ---> f(inner) ; ... .
There should be
:- type inner.
there too, otherwise that example shouldn't compile.
> :- implementation.
> :- type inner ---> inner( ... ).
> To solve the problem, the sub-module must be able to distinguish between type
> definitions which are available from the ancestor module's public interface,
> and type definitions which the ancestor only exported to sub-modules.
> The distinction was not apparent in the `.int0' private interface files --
> all items were contained into a single interface section, no matter
> where they came from in the original source file.
> This change separates `.int0' files into two sections: the interface section
> for items in the public interface, and the implementation section for items in
> the private interface.
> Change `.int0' files to have two sections as described.
> Sort the contents of the individual sections.
> Do not let other statuses override
> Fix the main bug described.
> Fix a bug where a type with `status_abstract_exported' was not
> considered exported-to-submodules, which it is.
> Rename and clarify that `import_locn_ancestor_private_interface_proper'
> really, really means that it came from the actual, true, and proper
> private interface of a module and nowhere else.
> Rename `md_private_interface' to what it really means.
> Conform to changes.
> Add test case.
> Update expected output.
> diff --git a/compiler/hlds_out_pred.m b/compiler/hlds_out_pred.m
> index 75c3f04..288ca69 100644
> --- a/compiler/hlds_out_pred.m
> +++ b/compiler/hlds_out_pred.m
> @@ -498,7 +498,7 @@ import_status_to_string(status_imported(import_locn_interface)) =
> import_status_to_string(status_imported(import_locn_implementation)) =
> "imported in the implementation".
> - import_locn_ancestor_private_interface)) =
> + import_locn_ancestor_private_interface_proper)) =
> "imported from an ancestor's private interface".
> import_status_to_string(status_imported(import_locn_ancestor)) =
> "imported by an ancestor".
> diff --git a/compiler/hlds_pred.m b/compiler/hlds_pred.m
> index 1d533fc..ffec4fc 100644
> --- a/compiler/hlds_pred.m
> +++ b/compiler/hlds_pred.m
> @@ -250,7 +250,8 @@
> % due to its presence in the .opt files
> % (intermod.adjust_pred_import_status).
> ; status_abstract_exported
> - % Describes a type with only an abstract declaration exported.
> + % Describes a type with only an abstract declaration exported
> + % to non-sub-modules. It *is* exported to sub-modules.
I would reword the last sentence to say:
The definition of the type is exported to sub-modules.
> ; status_pseudo_exported
> % The converse of pseudo_imported; this means that only the
> % (in, in) mode of a unification is exported.
> diff --git a/compiler/modules.m b/compiler/modules.m
> index 679ba68..fc62b30 100644
> --- a/compiler/modules.m
> +++ b/compiler/modules.m
> @@ -1540,22 +1551,78 @@ clause_in_interface_warning(ClauseOrPragma, Context) = Spec :-
> Spec = error_spec(severity_warning, phase_term_to_parse_tree,
> [simple_msg(Context, [always(Pieces)])]).
> - % strip_clauses_from_interface is the same as
> - % check_for_clauses_in_interface except that it doesn't issue any
> - % warnings, and that it also strips out the `:- interface' and `:-
> - % implementation' declarations.
> + % strip_clauses_private_interface is used when creating the private
> + % interface (`.int0') files for packages with sub-modules. It removes
> + % unnecessary items and separates interface and implementation items.
> + %
> + % The `.int0' file contains items which is available to any module in the
which *are* available ...
> + % interface section, and items which are only available to sub-modules in
> + % the implementation section. The term "private interface" is ambiguous:
> + % sometimes it refers to the `.int0' file which, as just explained,
> + % contains the public interface as well. The term "private interface
> + % proper" may be used to refer to the information in the implementation
> + % section of the `.int0' file.
> - % This is used when creating the private interface (`.int0') files for
> - % packages with sub-modules.
> + % (Historically, the `.int0' file did not distinguish between the public
> + % and private interfaces.)
That looks fine otherwise.
mercury-reviews mailing list
Post messages to: mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions: mercury-reviews-request at csse.unimelb.edu.au
More information about the reviews