Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings collection #2602

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Warnings collection #2602

wants to merge 11 commits into from

Conversation

hurufu
Copy link
Contributor

@hurufu hurufu commented Oct 5, 2024

After recent discussion #2599, I've decided to start collecting useful warnings that can be applied during compilation. If you have good ideas for warnings, please post them here.

Relevant issues: #2600 #2601.

If you want to test, just import warnings module:

:- use_module(library(warnings)).

And that's all.

@hurufu hurufu changed the title Warnings collection. Warnings collection Oct 5, 2024
@hurufu hurufu marked this pull request as draft October 5, 2024 12:31
src/lib/warnings.pl Outdated Show resolved Hide resolved
@hurufu
Copy link
Contributor Author

hurufu commented Oct 5, 2024

Tests are failing, because of duplicated warning message, I'll try to come up with minimal example.

UPDT: New issue: #2603

@hurufu
Copy link
Contributor Author

hurufu commented Oct 5, 2024

Currently singleton variables warning is done in loader.pl during goal expansion, if this PR will be accepted, I can move that code to library(warnings).

@triska
Copy link
Contributor

triska commented Oct 5, 2024

Conceptually very interesting! This is an aspect that I think would also generally benefit from more thought during development: Which parts of the system must be provided as foundation, and which can be built on top?

@hurufu
Copy link
Contributor Author

hurufu commented Oct 6, 2024

Ideas for warnings:

  • Warn if user tries to re-define built-in static predicate
  • Warn if a non-list is found eg.: [a,b,c|d].
  • Detect used, but not visible predicates + suggest which module to load (needs some additional support from loader)
  • Warn about duplicated cuts (!/0) in a single predicate body (can be quite tricky in the presence of lambdas)
  • Warn if predicate calls one of its arguments, but isn't declared as meta-predicate. Suggest correct meta-predicate declaration.
  • Warn when more than 2 negations are nested: \+ \+ \+ foo(X).. Double negation has legit use-case, but I don't think that more nested negations are useful.
  • Detect unsound type test predicates like integer/1 and suggest using library(si). (Thanks @triska)
  • Branch singleton variables warnings would be useful #2189
  • Warn about invalid arithmetic expressions

This list will be expanded in the future. Regarding implementation... if community thinks that this is a good idea, I will implement them. Currently I only gather ideas.

Important: those warnings will be optional, user is not required to import the library and see them.

@triska
Copy link
Contributor

triska commented Oct 6, 2024

One question that may be worth taking into account: What makes most programs wrong? I think the type tests integer/1, atom/1 are candidates for this: Their use makes many programs yield wrong answers. The safe type tests integer_si/1 etc. from library(si) could be automatically suggested in such cases.

src/lib/warnings.pl Outdated Show resolved Hide resolved
@triska
Copy link
Contributor

triska commented Oct 7, 2024

There seems to be a pattern in the expansion code, so maybe this code can be generated from shorter declarative descriptions of warnings?

@triska
Copy link
Contributor

triska commented Oct 7, 2024

#2189 may be a candidate to address with this technique.

@hurufu
Copy link
Contributor Author

hurufu commented Oct 7, 2024

I've added quite interesting and less trivial warning. It now detects missing meta_predicate declaration. I don't like how I implemented it (and I expect a lot of critique :) ), mainly because that hook predicate doesn't seem elegant to me. There is a lot of useful code in loader, but it is hard to use it in different contexts.

@triska
Copy link
Contributor

triska commented Oct 7, 2024

You may be able to find a better design for the loading process, to make adding such features more elegant!

@hurufu
Copy link
Contributor Author

hurufu commented Oct 8, 2024

I've copied code from #2458, that particular task should be fixed in the compiler itself, but Prolog code is useful enough to give it second life here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants