[m-rev.] for review: fix bugs involving unused foreign_proc args

[Julien Fischer]

On Wed, 12 Oct 2005, Zoltan Somogyi wrote:

> Fix the bugs were exposed by my pre-dummy-type-diff cleanup of polymorphism.m.
> This diff deleted a "semidet_fail" in the predicate that tested whether a
> namedforeign_proc used a variable, making the test a real test, not a dummy.
>
> The main bugs were in unused_args.m, polymorphism.m and ml_unify_gen.m.
>
> compiler/unused_args.m:
> 	Fix the mismatch between the analysis and fixup passes' treatment of
> 	foreign_procs. The analysis pass says that variables representing
> 	unnamed foreign_proc arguments are unused, and therefore the code
> 	generating them can be optimized away, but the fixup pass used to leave
> 	those arguments unchanged. This is usually fine, but there was a
> 	problem if a variable had two or more such conjoined consumers but no
> 	other consumers. It then had its generator optimized away, but the
> 	variable was left in the nonlocal set of each consumer, which meant
> 	that the code generator aborted when trying to save the value of the
> 	variable after the first consumer.
>
> 	The fix is to rename unnamed foreign proc args, ensuring that they are
> 	local to the foreign_proc they appear in. The code generators can
> 	already handle such arguments.
>
> compiler/typecheck.m:
> 	Detect foreign_procs that are supposed to bind type_info variables,
> 	but don't, and generate error messages for them. This is now necessary,
> 	since if accept such predicates, unused_args.m will now rename apart
if we accept

> 	the unused type_info argument, and the code generator will abort
> 	due to the type_info missing its generator.
>
> compiler/typecheck_errors.m:
> 	Add a predicate for reporting the error.
>
> compiler/goal_util.m:
> 	Add a utility predicate and export some previously internal predicates
> 	for use by unused_args.m.
>
> 	Delete redundant module prefixes on predicate names to avoid having to
> 	split long lines.
>
> compiler/polymorphism.m:
> 	Ensure that all arguments of import foreign_procs are considered used,
> 	since our contract with foreign language code prevents us from
> 	optimizing them away.
>
> 	Redefine a predicate using the new utility predicate in goal_util.m.
>
> compiler/pragma_c_gen.m:
> 	Ensure that all arguments of import foreign_procs are considered used,
> 	since our contract with foreign language code prevents us from
> 	optimizing them away. This is a belt-and-suspenders fix: the change to
> 	polymorphism.m should be sufficient to ensure this objective by itself.
>
> compiler/ml_unify_gen.m:
> 	The mark_static_terms pass was assuming that if X is a static term,
> 	then after the assignment unification Y = X Y is also a static term.
> 	However, when processing such assignments, the code generator here
> 	neglected to copy over the part of the X's state that says what static
> 	term it is to become part of Y's state. This diff fixes that neglect.
>
> compiler/ml_code_util.m:
> 	We used to identify MLDS variables holding static terms by a sequence
> 	number, and then create a name from the number by tacking on the name
> 	of the HLDS variable referring to it at each reference. This worked
> 	because each static term was referred to by only one HLDS variable;
> 	but after the diff to ml_unify_gen, this is no longer the case.
> 	We therefore now construct the MLDS variable's name when the static
> 	term is created, and use that name as the translation of all the HLDS
> 	variables referring to that static term.
>
> compiler/add_pragma.m:
> 	Minor cleanup.
>
> library/rtti_implementation.m:
> library/store.m:
> 	In some predicates that don't define type_infos for existentially typed
> 	arguments, mention the type_info that *should* be defined there in
> 	comments to avoid the error now generated by typecheck.m.
>
> browser/Mercury.options:
> deep_profiler/Mercury.options:
> 	Undo the workaround for the fixed bug.
>
> tests/existential_type_classes.m:
> 	In predicates that don't define type_infos for existentially typed
> 	arguments, mention the type_info that *should* be defined there in
> 	comments to avoid the error now generated by typecheck.m.
>
> tests/existential_type_classes.exp:
> 	Conform to the changed line numbers in existential_type_classes.m.
>
> tests/warnings/exist_foreign_error.{m,exp}:
> 	New test case for the error now detected by typecheck.m.
>
> tests/warnings/Mmakefile:
> 	Enable the new test case.
>

That's fine.

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------