[m-rev.] for review: delete obsolete procedures from string module
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.
More information about the reviews