It looks like some of the assertions may be actually valid as assertions. The remaining ones are:
1) in Closure constructor, parsing of parameters
else if (obj instanceof Cons) { if (state == STATE_AUX) { Symbol sym = checkSymbol(obj.car()); LispObject initForm = obj.cadr(); Debug.assertTrue(initForm != null); if (aux == null) aux = new ArrayList<Parameter>(); aux.add(new Parameter(sym, initForm, AUX)); }
I don't see how to create a null initform other than creating a Cons that has a null car or cdr. And I don't know how to do that from lisp code. To me, that looks like it requires serious abuse from the Java side to accomplish. And I don't think that would be the only place where the validity of a Cons would need to be checked. I'm inclined to just remove that assertion completely.
2) again in Closure constructor,
else { // Lambda list is empty. Debug.assertTrue(lambdaList == NIL); arity = 0; maxArgs = 0; }
That's an else branch of if (lambdaList instanceof Cons), and it looks like it asserts that our own code in Closure is correct. I'm inclined to keep that assertion as is.
3)
if (arity >= 0) Debug.assertTrue(arity == minArgs);
a couple of lines down, check that minArgs is equal to arity if arity isn't negative. Looks correct to me, I'm inclined to keep it as is.
4) if (optionalParameters.length == 0 && keywordParameters.length == 0) args = fastProcessArgs(args); else args = processArgs(args, thread); Debug.assertTrue(args.length == variables.length);
in the array version of execute. That looks like it verifies that our argument processing works, and looks correct to me. I'm inclined to keep it as is.
5) Parameter.processInitForm:
if (initForm.constantp()) { if (initForm instanceof Symbol) return initForm.getSymbolValue(); if (initForm instanceof Cons) { Debug.assertTrue(initForm.car() == Symbol.QUOTE); return initForm.cadr(); }
I'm not sure what is going on here. I don't know how to concoct a lambda that has a constant initform for a parameter so that the initform is Cons but is not quoted. Granted, I've mainly tested these changes with defuns, so I wouldn't be surprised if such a failure case can be demonstrated. I have no inclination here, this looks like it should probably be a ProgramError rather than an assert.
Please review these cases (and the patch itself) and comment. I ran the tests and I have a failure in MAKE-BROADCAST-STREAM.8 that my baseline doesn't have, but I'm not sure if it's related to these changes.
armedbear-devel@common-lisp.net