potassco / potassco/constraint-handler

Improvement of warnings.

Open
#86 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
3
Forks
0
Avg merge
1d 19h
Merged PRs (30d)
20

Description

here are some thoughts I had while looking at the warnings again.

If possible, I think the warnings should be streamlined into a common structure. Right now, warning/1 takes a tuple and can basically display anything, but I feel a little bit more structure could help.

Additionally, I think the statement identifiers are a good concept, but they aren't used consistently everywhere. For instance, for variable_declare I have to provide a statement identifier, but for variable_domain I don't. Instead, during the actual assignment the identifier of the declare is used. but the same doesn't hold true for set assignments where the user has to provide a statement identifier as well.

Given that a user technically can define a domain over multiple statements, the user may want to know which of these fails during an assignment. If instead the user shouldn't be allowed to spread a domain over multiple statements, then an identifier could be used to provide a meaningful message.

Likewise (as seen in variables.lp):

warning((X,"was defined twice:",E1,E2)) :- variable_define(CNAME1,X,E1), variable_define(CNAME2,X,E2),
  (CNAME1,E1) < (CNAME2,E2).

Here, I think E1 and E2 provide less value to the user than CNAME1 and CNAME2 given the user provides meaningful statement identifiers. E1 and E2 might be very complex expressions that are difficult to read while CNAME1 and CNAME2 can be used to easily verify duplication and identify the wrong statement.

The same goes for variable_declare where it can be even worse. Consider this:

variable_declare(some_id, y, fromFacts). % In one file
variable_declare(some_id_2, y, fromFacts). % In some other file

The user would receive the warning warning((y,"was declared twice:",fromFacts,fromFacts)) which doesn't help much. If the user instead received warning((y,"was declared twice:", some_id, some_id_2)) they could find the specific declarations immediately.

Furthermore, I feel that right now a user would have to know very specific error messages in order to identify or filter for specific errors. While this is probably not as common here, it may still help to instead provide names of error types. Otherwise, re-wording of messages may break a user's filter even though the error type and content otherwise remains the same.

Generally, if possible, I would suggest to always propagate the statement names, use statement names everywhere and then do something like:

warning(Type, Identifier, Details).
Name Description
Type The type of error/warning that was issued.
Identifier The identifier of the statement that caused the warning.
Details Here we could put the details like before.

We could then have types such as duplicate_variable_definition_error,duplicate_variable_declaration_error or expression_evaluation_error (or use whatever naming-convention you like). A user can then use these to filter for certain error types without relying on the specific wording of the error messages.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing warning/1 and statement-identifier propagation across the warning implementations, including variable.lp and the variable_declare, variable_domain, and set-assignment cases mentioned here. Define the desired structured warning fields and identifier behavior, then verify that duplicate declarations and definitions expose the useful statement identifiers and warning types.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.