[m-rev.] for review: --warn-suspicious-foreign-procs

[Peter Wang]

On 2009-09-05, Julien Fischer <juliensf at csse.unimelb.edu.au> wrote:
> 
> Add a new warning, --warn-suspicious-foreign-procs.
> This checks the bodies of foreign_proc pragmas for possible errors.
> Currently, these errors are:
> 
> (1) the presence of SUCCESS_INDICATOR (or in Java, succeeded),
> in foreign procs. for predicates that cannot fail.
> 
> (2) the lack of SUCCESS_INDICATOR (succeeded) in foreign procs.
> for predicates that can fail.
> 
> (3) the presence of "return" (or "ret" or "jmp" in IL).  This could
> indicate the presence of a return statement in the foreign proc.
> 
> Potentially, we could also check for the presence of the this
> pointer in Java and C# foreign_procs, but that isn't implemented
> at the moment.
> 
> The warning is disabled by default, since it will produce spurious
> results if the things it checks for occur in foreign language
> comments.

In the case of (3) this could occur quite often which might limit
uptake of the option.  Which would be a shame as the success indicator
checks should be safe and useful to enable by default.

> Index: compiler/make_hlds_warn.m
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/compiler/make_hlds_warn.m,v
> retrieving revision 1.32
> diff -u -r1.32 make_hlds_warn.m
> --- compiler/make_hlds_warn.m	2 Sep 2009 00:30:16 -0000	1.32
> +++ compiler/make_hlds_warn.m	4 Sep 2009 14:31:22 -0000

> @@ -608,9 +611,178 @@
>      varset.search_name(VarSet, Var, Name),
>      string.prefix(Name, "_").
> 
> +:- pred pragma_foreign_proc_body_checks(foreign_language::in,
> +    prog_context::in, simple_call_id::in, pred_id::in, proc_id::in,
> +    module_info::in, list(string)::in,
> +    list(error_spec)::in, list(error_spec)::out) is det.
> +
> +pragma_foreign_proc_body_checks(Lang, Context, PredOrFuncCallId, PredId, ProcId,
> +        ModuleInfo, BodyPieces, !Specs) :-
> +    check_fp_body_for_success_indicator(Lang, Context, PredOrFuncCallId,
> +        PredId, ProcId, ModuleInfo, BodyPieces, !Specs),
> +    check_fp_body_for_return(Lang, Context, PredOrFuncCallId, BodyPieces,
> +        !Specs).

Wouldn't these be better as a single pass? (not necessarily in this commit)

> +
> +    % Check to see if a foreign_proc body contains a return statment

statement

> Index: doc/user_guide.texi
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/doc/user_guide.texi,v
> retrieving revision 1.592
> diff -u -r1.592 user_guide.texi
> --- doc/user_guide.texi	3 Sep 2009 23:57:44 -0000	1.592
> +++ doc/user_guide.texi	4 Sep 2009 15:01:07 -0000
> @@ -6313,6 +6313,11 @@
>  @findex --no-warn-unresolved-polymorphism
>  Do not warn about unresolved polymorphism.
> 
> + at sp 1
> + at item --warn-suspicious-foreign-procs
> + at findex --warn-suspicious-foreign-procs
> +Warn about possible errors in the bodies of foreign procedures.
> +

This should describe the checks as currently implemented,
or at least the limitations of the implementation.

Peter
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------