Fix DEFPACKAGE, reader #:, APROPOS, and DISASSEMBLE ANSI test failures#743
Fix DEFPACKAGE, reader #:, APROPOS, and DISASSEMBLE ANSI test failures#743blakemcbride wants to merge 3 commits intoarmedbear:masterfrom
#:, APROPOS, and DISASSEMBLE ANSI test failures#743Conversation
|
@blakemcbride Thanks for the rush of pull requests. One request though: if you have multiple pull requests outstanding that are not orthogonal (i.e. some add further changes that depend on previous requests) could you please close "older" pull requests possibly with a comment/link to the superseding one? This will help me understand the totality of what you are requesting much better. Overall, the quality of the patches seems pretty decent. Given the sudden burst of requests, I presume you are using some computer-assisted augmentation? May I ask what sort? |
|
I apologize for the overlapping patches. Truth be told, I didn't realize I did that until after I submitted the pull request. I sort of ran out of time to fix it and submit new pull requests. Your help in untangling and using these pull requests is greatly appreciated. I'll be more careful in the future. I used Claude Code. I have been using it a lot lately. Although it isn't perfect, it is as good as most developers I've worked with. It makes mistakes as developers do. You test it and give feedback rather than fire them because they're not infallible. There have been many projects I have been interested in over many years. (See my GitHub repo.) I have wanted to do many things with these packages but just didn't have the time. Now, with Claude Code, I do. My hope was to correct all ABCL ANSI Standard test issues. Collectively, the patches I provided fixed a good percentage of the issues. My plan is that if the patches are accepted, I will finish the project. It is basically up to your team if I continue. I have been using ABCL for many years as an extension language. I even paid a member of your team to make corrections in the system years ago. I think ABCL is a wonderful system. I would like to contribute to its success. Thank you! |
So, if I see things overlapping, I will close with an appropriate comment. This won't necessarily mean rejection for the pull request, as we can debate, revise, and re-open as needed. |
|
I see what is going on. I will close the two pull requests that are duplicates. Thanks. |
|
I closed 741 and 742 since they both exist in 743. All should be straight now. Thanks! |
|
The descriptions in 741 & 742 contain descriptions of the earlier commits in this commit. |
|
I will wait on the merge of these commits in order to continue the work I was doing. |
3. APROPOS / APROPOS-LIST arity (
|
|
Superseded by #748 |
|
@blakemcbride Spent some time today working through these changes; it's gonna take me a lot longer to review than I initially thought. I feel now like John Henry had a grudging respect for the steam engine. It would be nice if you tried to urge your augmentation to make smaller commits that make atomic, testable changes. From quick reviews of what you opened today (Tue Apr 21), it seems like you are working on honing your requests in that manner. Also, it would be great if you put the comments from the Github request into the commit message itself, as Github won't always be around but the commit messages will be. If you open a pull request for a single commit, Github automatically copies the commit message as the starting "Conversation" comments. |
|
I can certainly do that in the future. Are you requesting that I do that for the pull requests that I have already done? Incidentally, taking all of my pull requests together makes ABCL pass 100% of the standard ANSI CL tests. |
|
You closed this pull request. Did you merge it? |
Fix DEFPACKAGE, reader
#:, APROPOS, and DISASSEMBLE ANSI test failuresSummary
Four targeted ANSI-conformance fixes across the package, reader,
apropos, and disassemble subsystems. All originally-targeted tests
pass, and no previously-passing test regresses.
DEFPACKAGE.2BSYNTAX.SHARP-COLON.ERROR.1APROPOS.ERROR.2APROPOS-LIST.ERROR.2DISASSEMBLE.ERROR.3Changes
1. DEFPACKAGE
:nicknames— accumulate instead of overwrite (defpackage.lisp)CLHS specifies that multiple
:nicknamesoptions are merged(
defpackage"Syntax" under §11.1.2). The ABCL macro was usingsetq, so only the last:nicknamesclause survived:Fixed by appending the new names to the accumulator, matching the
handling of
:shadow,:export, etc.2. Reader
#:token — signal on unescaped package markers (Stream.java)CLHS 2.4.8.7 requires
#:<token>to signal a reader error if thetoken contains an unescaped package marker (a colon not preceded by
\or inside a|…|escape). ABCL previously accepted#:foo:barsilently and constructed a symbol whose name contained a colon.
Stream.readSymbolnow scans the token post-_readToken, consultingthe per-character escape
BitSetreturned by the tokenizer, andsignals
ReaderErroron any unescaped:. The check is skippedwhen
*read-suppress*is true, soREAD-SUPPRESS.SHARP-COLON.7continues to pass (it reads
"#::"with*read-suppress*= T andexpects
NIL, no error).3. APROPOS / APROPOS-LIST arity (
apropos.lisp)CLHS defines
aproposandapropos-listwith the lambda-list(string-designator &optional package-designator)— two args max.ABCL exposed a third
external-onlyextension parameter, so theconformance tests calling them with three arguments (expecting
PROGRAM-ERROR) failed.Renamed the internal three-arg helper to
%apropos-listandreinstated CLHS-conforming two-arg signatures for the public
aproposandapropos-list. Theexternal-onlyfunctionality isretained for internal callers via
%apropos-list.4. DISASSEMBLE type-error on bad argument (
disassemble.lisp)CLHS specifies
disassemble's argument as an extended functiondesignator — i.e.
(or function symbol (cons (eql setf) (cons symbol null)))— and mandates aTYPE-ERRORotherwise.DISASSEMBLE.ERROR.3iterates the mini-universe expectingTYPE-ERRORfor everything outside that set, but ABCL acceptedarbitrary junk, often crashing inside
disassemble-function.Added an explicit
typepguard that signalsTYPE-ERRORwith:datumand:expected-typepopulated. The accepted type is widenedwith
(cons (eql lambda) t)so thatDISASSEMBLE.3(
(disassemble '(lambda (x y) (cons y x))), which expects success)keeps passing.
Files changed
src/org/armedbear/lisp/defpackage.lisp—:nicknamesaccumulate.
src/org/armedbear/lisp/Stream.java—readSymbolunescaped-coloncheck, guarded by
*read-suppress*.src/org/armedbear/lisp/apropos.lisp— public arity restored;internal helper renamed to
%apropos-list.src/org/armedbear/lisp/disassemble.lisp—typepguard ondisassemblearg, withlambda-expression allowance.Test plan
ant abclbuilds cleanly.DEFPACKAGE.2B,SYNTAX.SHARP-COLON.ERROR.1,APROPOS.ERROR.2,APROPOS-LIST.ERROR.2,DISASSEMBLE.ERROR.3.READ-SUPPRESS.SHARP-COLON.7(reader),DISASSEMBLE.3(lambda-expression acceptance).asdf:test-system :abcl/test/ansi/compiled.Compatibility
No public API breakage.
APROPOS/APROPOS-LISTbecome strictlyCLHS-conforming: callers relying on ABCL's undocumented three-arg
form should switch to
SYS::%APROPOS-LISTor filter externally.Reader behavior for well-formed
#:tokens is unchanged; only thepreviously-accepted malformed case now errors as CLHS requires.