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

Split up inconsistent entry error #2463

Merged
merged 11 commits into from
Aug 5, 2024

Conversation

Skgland
Copy link
Contributor

@Skgland Skgland commented Aug 3, 2024

I got annoyed at the unhelpful nature of the inconsistent entry error, while not having a line number for the source of the error makes it still somewhat unhelpful at least different error reasons are now separated.

The way these errors are presented is probably still incorrect as having them inside a syntax error is probably wrong. Maybe someone else can give pointers how to make this correct or just take over.

Thanks to triska for the quotes from the standard.

@triska
Copy link
Contributor

triska commented Aug 3, 2024

Thank you so much for working on this!

A syntax error is indeed not applicable here (this is also currently wrongly handled, so not specific to your PR!), because syntactically, everything is correct.

The ISO standard defines which errors to throw:

8.14.3 op/3

A goal op(Priority, Op_specifier, Operator) en-
ables the operator table (see 6.3.4.4 and table 7) to be
altered.

...

8.14.3.3 Errors

  a) Priority is a variable
  - instantiation_error.

  b) Op-specifier is a variable
  - instantiation_error.

  c) Operator is a partial list or a list with an element E
  which is a variable
  - instantiation_error.

  d) Priority is neither a variable nor an integer
  - type_error(integer, Priority).

  e) Op_specifier is neither a variable nor an atom
  - type_error(atom, Op_specifier).

  f) Operator is neither a partial list nor a list nor an atom
  - type_error(list, Operator).

  g) An element E of the Operator list is neither a
  variable nor an atom
  - type_error(atom, E).

  h) Priority is not between 0 and 1200 inclusive
  - domain_error(operator_priority, Priority).

  i) Op_specifier is not a valid operator specifier
  - domain_error(operator_specifier, Op_specifier).

  j) Operator is ','
  - permission_error(modify, operator, ',').

  k) An element of the Operator list is ','
  - permission_error(modify, operator, ',').

  l) Op_specifier is a specifier such that Operator
  would have an invalid set of specifiers (see 6.3.4.3)
  - permission_error(create, operator, Operator).

@Skgland
Copy link
Contributor Author

Skgland commented Aug 3, 2024

I assumed that it was defined in the standard, but I don't have that at hand. I have now adjusted the errors based on you cited excerpt from the standard, but couldn't figure out how to include the incorrect term in case of the type errors, using todo_insert_invalid_term_here as a stand in for now.

See the declaration_errors.md for the current errors.

@triska
Copy link
Contributor

triska commented Aug 3, 2024

There is also Technical Corrigendum 2, notably:

There shall not be an operator '{}' or '[]'.
An operator '|' shall be only an infix operator with priority greater than or equal to 1001.

@triska
Copy link
Contributor

triska commented Aug 3, 2024

For the case of directives that are not supported, a domain error seems applicable: domain_error(directive, ...).

An existence error is only thrown when an operation is performed, such as when execution of a procedure is attempted.

@Skgland Skgland force-pushed the split-up-inconsitent-entry-error branch from 3a833f9 to c8a4d65 Compare August 3, 2024 21:02
@Skgland
Copy link
Contributor Author

Skgland commented Aug 3, 2024

Ok, updated errors at declaration_errors.md

@triska
Copy link
Contributor

triska commented Aug 4, 2024

Thank you a lot, this is already infinitely better than what we have now!

I have one remaining small issue: In :-(D)., D is not called a "declaration", but a directive:

7.4.2 Directives

The characters of a directive-term in Prolog text (6.2.1.1)
shall satisfy the same constraints as those required to input
a read-term during a successful execution of the built-in
predicate read_term/3 (8.14.1). The principal functor
shall be (:-)/1, and its argument shall be a directive.
...

@Skgland
Copy link
Contributor Author

Skgland commented Aug 4, 2024

All instances of declaration that should be directive are now replaced.

@triska
Copy link
Contributor

triska commented Aug 4, 2024

Thank you a lot, it looks excellent! The only remaining issue I see is that directive is not a type, and therefore, as mentioned in #2463 (comment), a domain error seems more appropriate if a directive that is not supported is encountered.

@Skgland
Copy link
Contributor Author

Skgland commented Aug 4, 2024

Thank you a lot, it looks excellent! The only remaining issue I see is that directive is not a type, and therefore, as mentioned in #2463 (comment), a domain error seems more appropriate if a directive that is not supported is encountered.

Ah, I took that referenced comment as only pointing out the existence_errors as a wrong.

The instance where the type_error is returned feels less of an unsupported directive and more like not a directive at all.
Which appears to me closer to a string is not an int (i.e. wrong type) than 64 is not in the range 0-32 (i.e. wrong domain).

It is only issued when the term we expect to be a directive isn't a clause term.
The tests that results in this error invalid_decl6.pl is

:- 9001.

which appears to me to be an incorrect type. Unless an integer can somehow be a directive?

@triska
Copy link
Contributor

triska commented Aug 4, 2024

A type error indicates a valid (expected) type, and directive is not defined as a valid type:

7.12.2 Error classification

...

  b) There shall be a Type Error when the type of an
  argument or one of its components is incorrect, but not
  a variable. It has the form type_error(ValidType, Culprit)
  where

      ValidType E {
        atom,
        atomic,
        byte,
        callable,
        character,
        compound,
        evaluable,
        in_byte,
        in_character,
        integer,
        list,
        number,
        predicate_indicator,
        variable
      }.

  and Culprit is the argument or one of its components
  which caused the error.

In fairness, directive is also not a defined domain:

  c) There shall be a Domain Error when the type of an
  argument is correct but the value is outside the domain
  for which the procedure is defined. It has the form
  domain_error(ValidDomain, Culprit) where

     ValidDomain E {
       character_code_list,
       close_option, 
       flag_value,
       io_mode,
       non_empty_list,
       not_less_than_zero,
       operator_priority,
       operator_specifier,
       prolog_flag,
       read_option,
       source_sink,
       stream,
       stream_option,
       stream_or_alias,
       stream_position,
       stream_property,
       write_option
     }.

  and Culprit is the argument or one of its components
  which caused the error.

Still, a domain error seems somewhat more appropriate because D is a directive when we write :- D., so in a sense it does have the right "type", it's only outside the supported set (domain) of terms that can appear in place of D.

Note that if D is a variable, an instantiation error seems most appropriate. Currently, we get:

?- [user].
:- D.
loops, unexpected.
instantiation_error. % expected

@triska
Copy link
Contributor

triska commented Aug 4, 2024

For reference, regarding definitions related to directives:

3.57 directive: A term D which affects the meaning of
Prolog text (see 7.4.2), and is denoted in that Prolog text
by a directive-term :-(D).

3.58 directive-term: A read-term T. in Prolog text
where T has principal functor (:-)/1 (see 6.2.1.1).

And:

6.2.1.1 Directives

           directive term = term, end ;
Abstract:  dt               dt
Priority:                   1201
Condition: The principal functor of dt is (:-)/1

           directive = directive term ;
Abstract:  d           :-(d)

NOTE - Subclause 7.4.2 defines the possible directives and
 their meaning.

@Skgland
Copy link
Contributor Author

Skgland commented Aug 4, 2024

A type error indicates a valid (expected) type, and directive is not defined as a valid type:

7.12.2 Error classification

...

b) There shall be a Type Error when the type of an
argument or one of its components is incorrect, but not
a variable. It has the form type_error(ValidType, Culprit)
where

  ValidType E {
    atom,
    atomic,
    byte,
    callable,
    character,
    compound,
    evaluable,
    in_byte,
    in_character,
    integer,
    list,
    number,
    predicate_indicator,
    variable
  }.

and Culprit is the argument or one of its components
which caused the error.
In fairness, directive is also not a defined domain:

pair and tcp_listener are also used as types in scryer-prolog and not on this list.

c) There shall be a Domain Error when the type of an
argument is correct but the value is outside the domain
for which the procedure is defined. It has the form
domain_error(ValidDomain, Culprit) where

 ValidDomain E {
   character_code_list,
   close_option, 
   flag_value,
   io_mode,
   non_empty_list,
   not_less_than_zero,
   operator_priority,
   operator_specifier,
   prolog_flag,
   read_option,
   source_sink,
   stream,
   stream_option,
   stream_or_alias,
   stream_position,
   stream_property,
   write_option
 }.

and Culprit is the argument or one of its components
which caused the error.

Here order has been added.

Still, a domain error seems somewhat more appropriate because D is a directive when we write :- D., so in a sense it does have the right "type", it's only outside the supported set (domain) of terms that can appear in place of D.

Sure, I will change it to a domain error.

Note that if D is a variable, an instantiation error seems most appropriate. Currently, we get:

?- [user].
:- D.
loops, unexpected.
instantiation_error. % expected

Yeah I think I already ran into that with invalid_decl11.pl for which I have disabled the added test as it hung.

For reference, regarding definitions related to directives:

3.57 directive: A term D which affects the meaning of
Prolog text (see 7.4.2), and is denoted in that Prolog text
by a directive-term :-(D).

3.58 directive-term: A read-term T. in Prolog text
where T has principal functor (:-)/1 (see 6.2.1.1).
And:

6.2.1.1 Directives

       directive term = term, end ;

Abstract: dt dt
Priority: 1201
Condition: The principal functor of dt is (:-)/1

       directive = directive term ;

Abstract: d :-(d)

NOTE - Subclause 7.4.2 defines the possible directives and
their meaning.

Yeah, that makes it sound like any term might be a directive i.e. correct type so I will change it to an domain error.

@triska
Copy link
Contributor

triska commented Aug 4, 2024

pair and tcp_listener are also used as types in scryer-prolog and not on this list.
Here order has been added.

Yes indeed, well spotted! pair and order were added in Technical Corrigendum 2:

Remove in subclause b variable from the enumerated set ValidType and add pair to the set ValidType. Add in subclause c order to the set ValidDomain.

@triska
Copy link
Contributor

triska commented Aug 4, 2024

Perfect, thank you a lot! I have filed #2467 for the remaining issue we found, which is independent from what you solved here so nicely! With your changes, the errors are now vastly improved. Thank you a lot!

@Skgland Skgland force-pushed the split-up-inconsitent-entry-error branch from 122225c to 058b9c5 Compare August 5, 2024 19:28
@Skgland
Copy link
Contributor Author

Skgland commented Aug 5, 2024

Rebased to resolve conflict with #2466

@mthom mthom merged commit 7cfe8ee into mthom:master Aug 5, 2024
13 checks passed
@Skgland Skgland deleted the split-up-inconsitent-entry-error branch August 5, 2024 20:33
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.

3 participants