Error when compiling call to SB-INT:MEMQ on a dotted list

[Paul F. Dietz]

(compile nil '(lambda (x) (sb-int:memq x '(a b . c))))

==>

The value
  C
is not of type
  LIST
   [Condition of type TYPE-ERROR]
[...]
  0: ((LABELS SB-C::REC :IN SB-INT:MEMQ) C)
  1: ((LABELS SB-C::REC :IN SB-INT:MEMQ) (B . C))
  2: ((LABELS SB-C::REC :IN SB-INT:MEMQ) (A B . C))
  3: ((SB-C:DEFTRANSFORM SB-INT:MEMQ) #<SB-C::COMBINATION :FUN #<SB-C::REF :LEAF #<SB-C::GLOBAL-VAR :%SOURCE-NAME SB-INT:MEMQ :TYPE #1=#<SB-KERNEL:FUN-TYPE #> :DEFINED-TYPE #1# :WHERE-FROM :DECLARED :KIND..
  4: (SB-C::IR1-OPTIMIZE-COMBINATION #<SB-C::COMBINATION :FUN #<SB-C::REF :LEAF #<SB-C::GLOBAL-VAR :%SOURCE-NAME SB-INT:MEMQ :TYPE #1=#<SB-KERNEL:FUN-TYPE #> :DEFINED-TYPE #1# :WHERE-FROM :DECLARED :KIND ..
  5: (SB-C::IR1-OPTIMIZE #<SB-C:COMPONENT :NAME (LAMBDA (X)) {1021A5CB73}> NIL)
  6: (SB-C::IR1-OPTIMIZE-UNTIL-DONE #<SB-C:COMPONENT :NAME (LAMBDA (X)) {1021A5CB73}>)
  7: (SB-C::IR1-OPTIMIZE-PHASE-1 #<SB-C:COMPONENT :NAME (LAMBDA (X)) {1021A5CB73}>)
  8: (SB-C::IR1-PHASES #<SB-C:COMPONENT :NAME (LAMBDA (X)) {1021A5CB73}>)

[Christophe Rhodes (csr21-cantab)]

 status confirmed
 assignee csr21-cantab

Thanks. Possibly this isn't triggered by user code without an explicit
call to an internal function, but we shouldn't fail in any case. Patch
attached.

[Douglas Katzman (dougk)]

That's not how I would "fix" this, if I were to. The transform's constraint should be (constant-arg (satisfies proper-list-p)) and proper-list-p should be made foldable.

However, to me this is a won't-fix and I feel pretty strongly about that.
It is no different from https://bugs.launchpad.net/sbcl/+bug/1887323
which says "It is not to be used on random stuff"

Also there's no end to the list of things that can crash when you hand the compiler a form that explicitly mentions system internals
* (compile nil '(Lambda (x) (%primitive nope)))
debugger invoked on a BUG in thread
#<THREAD "main thread" RUNNING {10015A8143}>:
    undefined primitive NOPE
  This is probably a bug in SBCL itself ...

[Paul F. Dietz (paul-f-dietz)]

My feeling on this: if a symbol is exported "officially" for use by users in their programs, it should work. And if the symbol is not intended for use by users, it should not be external in that internal package.

The use came from PVS, btw, where three packages shadowing-import it from sb-int.

[Stas Boukarev (stassats)]

sb-int is not an external package, as the "int" implies.

[Christophe Rhodes (csr21-cantab)]

"Paul F. Dietz" <email address hidden> writes:

> My feeling on this: if a symbol is exported "officially" for use by
> users in their programs, it should work. And if the symbol is not
> intended for use by users, it should not be external in that internal
> package.

Well, that can't be right. SBCL has a right to document packages as
being for internal use, and to export symbols from such packages for its
own internal use. The docstring of SB-INT states (as its first word)
that it is private.

> The use came from PVS, btw, where three packages shadowing-import it
> from sb-int.

The fact that users use it (and perhaps despite it being in a private
package, that it has a documented heritage from CMUCL's extensions)
indicate to me that there is benefit to being robust to potential user
misuse.

[Christophe Rhodes (csr21-cantab)]

Douglas Katzman <email address hidden> writes:

> That's not how I would "fix" this, if I were to. The transform's
> constraint should be (constant-arg (satisfies proper-list-p)) and
> proper-list-p should be made foldable.

Yeah, I was trying to avoid proliferating the number of calls to
proper-list-p (there's already one in memq-translation-as-case), but
probably that's not worth bothering about.

> However, to me this is a won't-fix and I feel pretty strongly about
> that.

Strongly enough to veto a change? Strongly enough that, if it had been
written as in my patch in the first place, you would take out the checks
and introduce the error at compile-time?

[Stas Boukarev (stassats)]

The defknown is already

(defknown (memq assq) (t proper-list) list (foldable flushable))

I guess we should make it perform the check for propriety before applying any transforms. (Compare the behavior with assq, which doesn't have a transform).

[Stas Boukarev (stassats)]

Which would be

[Paul F. Dietz (paul-f-dietz)]

How hard would it be to manually import specific symbols between internal packages in SBCL, rather than :export then :use? That would at least force naughty users to deliberately import specific symbols or use ::.

BTW, there are 358 occurrences of sb-int: in the quicklisp dist (not including uses in ; comments), so even though use of those symbols is discouraged it still happens widely. I can put together a list of these symbols if that would be of interest.

[Stas Boukarev (stassats)]

> That would at least force naughty users

Nothing will stop them.

[Paul F. Dietz (paul-f-dietz)]

These are the 62 exported sb-int symbols in quicklisp:

(ATTEMPT-RESYNC AVER BASE-CHAR-CODE-LIMIT BINDING* BIT-VECTOR-=
 BOOTSTRAP-PACKAGE-NOT-FOUND CLEAR-INFO COLLECT CONSTANT-FORM-VALUE
 DEBOOTSTRAP-PACKAGE DEFCONSTANT-EQX DOVECTOR ENCAPSULATE
 ENCAPSULATED-CONDITION ENCAPSULATED-P *EOF-OBJECT* FEATUREP FIXNUMP
 GET-FLOATING-POINT-MODES GLOBAL-FTYPE INDEX INFO KEYWORDICATE LEGAL-FUN-NAME-P
 LIST-OF-LENGTH-AT-LEAST-P MAKE-MACRO-LAMBDA MEMQ MIX &MORE NAMED-LAMBDA
 NAMED-LET ONCE-ONLY PACKAGE-AT-VARIANCE PACKAGE-EXTERNAL-SYMBOL-COUNT
 PACKAGE-INTERNAL-SYMBOL-COUNT *PRINT-CONDITION-REFERENCES*
 PROPER-LIST-OF-LENGTH-P QUASIQUOTE REFERENCE-CONDITION
 REFERENCE-CONDITION-REFERENCES *REPL-PROMPT-FUN* *REPL-READ-FORM-FUN*
 SANE-PACKAGE SET-FLOATING-POINT-MODES SIMPLE-COMPILER-NOTE SIMPLE-FILE-ERROR
 SIMPLE-PARSE-ERROR SIMPLE-PROGRAM-ERROR SIMPLE-READER-ERROR
 SIMPLE-READER-PACKAGE-ERROR SIMPLE-STREAM-ERROR SIMPLE-STYLE-WARNING
 STREAM-DECODING-ERROR STREAM-ENCODING-ERROR STRERROR SYMBOLICATE TYPE-WARNING
 UNENCAPSULATE UNSUPPORTED-OPERATOR VALID-FUNCTION-NAME-P
 WITH-FLOAT-TRAPS-MASKED WITH-UNIQUE-NAMES)

It might be useful to determine which of these, if any, should be officially exposed as part of the SBCL extension API.

[Richard M Kreuter (kreuter)]

If anyone's collecting a list of worthwhile package export clarifications/cleanups, ISTM SB-SYS's documented "private"-ness is fishy.

1. The manual references several symbols external in SB-SYS (timeouts, deadlines, interrupts, MAKE-FD-STREAM, WITH-PINNED-OBJECTS, WITHOUT-GCING), and
2. advises the user to look for SAP functionality in SB-SYS.

Additionally, I've found on a few other symbols external to SB-SYS to be useful in my programs over the years, despite knowing that the package is supposed to be private. (Shame on me.)

3. *STDIN*, *STDOUT*, *STDERR*, *TTY*,
4. SERVE-EVENT and its helpers,
5. BEEP.

While there are some external symbols in SB-SYS that I'd say really are internals, most of the externals seem to be supported or at least semi-supported extensions.

[Christophe Rhodes (csr21-cantab)]

Re-exporting SAP, SERVE-EVENT, and some things from SB-INT from SB-EXT is reasonable, given that we haven't actually changed their interfaces much for ~20 years; it's orthogonal to this issue, though.