Split up inconsistent entry error

[Skgland]

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]

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]

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]

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]

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]

Ok, updated errors at declaration_errors.md

[triska]

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]

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

[triska]

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 https://github.com/mthom/scryer-prolog/pull/2463#issuecomment-2267133168, a domain error seems more appropriate if a directive that is not supported is encountered.

[Skgland]

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]

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]

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]

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]

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]

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]

Rebased to resolve conflict with #2466