[m-rev.] for review: delete obsolete procedures from string module

Julien Fischer jfischer at opturion.com
Wed Apr 13 22:57:40 AEST 2022


On Wed, 13 Apr 2022, Zoltan Somogyi wrote:

>
> 2022-04-13 18:10 GMT+10:00 "Peter Wang" <novalazy at gmail.com>:
>>> One: make this new warning conditional on a new option, and disable this option
>>> in modules where the new warning would cause compilation to fail through
>>> --halt-at-warn. This would disable the warning in some of the modules where
>>> we could benefit from it, such as string.m itself.
>>>
>>> Two: delete the modes from clauses for predicates with foreign_procs.
>>>
>>> Three: do not generate the warning for predicates that also have foreign_procs.
>>> This cannot be done with the current architecture of make_hlds, because we add
>>> all clauses before we add any foreign_procs.
>>>
>>> Four: the status quo, with no new warning at all.
>>>
>>> I prefer two. Opinions?
>>
>> I agree with two.
>>
>> Since this is a style warning, there should be an option to turn it off,
>> implied by --inhibit-style-warnings.
>
> The attached diff does this. For review by anyone.

The diff looks fine.

> The diff includes a new test case, but since it turns out that an
> existing test also gets the new warning, I don't think we need it.
> Does anyone else?

That depends, should tests in tests/invalid be intentionally checking
for warnings?  There are tests in that directory that emit warnings,
I haven't looked at whether those warnings are germane to what is being
tested.  However, my feeling is that specific tests for warnings ought
to go into tests/warnings.  I would be inclined to disable the new
warning for the test in tests/invalid and retain the specific test
case in tests/warnings.

Julien.


More information about the reviews mailing list