define-object-amendment affects source definition locations

[reiniervandijk]

When using define-object-amendment, any use of M-. (find-tag) for that class symbol will jump to the buffer where amendment was made. It is impossible (as far as I know) to retrieve the original definition. Shouldn't M-. point to original definition instead of pointing to an amendment?

[genworks]

Good point. The define-object-amendment mechanism needs to be hooked into slime-edit-definition. As it is currently the define-object-amendment does a rather brute-force redefinition of the class object.

[genworks]

Open for submissions: $200~~-300~~. Requires full regression tests in the style of the tests in gendl/regression/source/ and e.g. gendl/regression/geom-base/source/angle-between-vectors.lisp.

~~define-object-amendment also needs to be audited to make sure it is up to date with the latest define-object.~~

[genworks]

Merging #167 into here:

Macro define-object-amendment does not check for override of reserved words. Hence, a user can naively break gendl completely by doing something like.

(define-object foo ())

And then:

(define-object-amendment foo ()
  :computed-slots
  ((parent nil)))

[reiniervandijk]

Some questions:

1. I have made a fix for this assuming you wanted to maintain the brute-force redefinition approach. Is this presumption okay? Or do you fundamentally want to change the amendment implementation? At this stage I had to add a slot to the metaclass (to keep a pointer to the original class which is redefined) and make some extensions to the swank-backend. This works now.
2. Do you prefer M-. to prompt a list of files, one for the base definition and one for each amendment. Or do you only want it to work with the original class definition only? In the latter case, M-. immediately takes the user to the definition in the corresponding file. Otherwise, a new menu prompts the user to choose which file to go to. My preference would go to the latter, but both are fine to me, your call.
3. I just notice a new part of the problem assignment: "define-object-amendment also needs to be audited to make sure it is up to date with the latest define-object." This is a rather vague assignment. Actually, so vague, that it's hard to see what more is expected from me here to solve the original source redefinition issue.
4. What do you expect to see at: "Requires full regression tests"? Your tests seem to not actually test for equality or anything or for the fact that some error must be raised, instead, it just checks if the regression-test-data slot runs without failure. Do you expect me to write some code in this slot that implement a true test, e.g. if source-files for new class is not equal to some-known-value then raise.

If you could provide a short-term answer to the former questions, then I'm ready to upload.

[reiniervandijk]

Status update:

• source locations maintained for original object and each amendment to it
• reserved word check implemented
• regression-test made (with best assumptions on what you expect to see)

[genworks]

I have made a fix for this assuming you wanted to maintain the brute-force redefinition approach. Is this presumption okay? Or do you fundamentally want to change the amendment implementation?

I'm not sure of any other way to do it, without getting into internal (and probably non-portable) CLOS and MOP tricks.

At this stage I had to add a slot to the metaclass (to keep a pointer to the original class which is redefined) and make some extensions to the swank-backend. This works now.

Do you mean a pointer to the file or to the actual class object itself?

Does this work for the case where multiple define-object-amendments are invoked?

Do you prefer M-. to prompt a list of files, one for the base definition and one for each amendment. Or do you only want it to work with the original class definition only?

I would think it should present a list of all locations of definitions - similar to what you get for a defmethod which has several implementations for different argument signatures.

I just notice a new part of the problem assignment: "define-object-amendment also needs to be audited to make sure it is up to date with the latest define-object." This is a rather vague assignment. Actually, so vague, that it's hard to see what more is expected from me here to solve the original source redefinition issue.

Please forget that part of the assignment, and we'll stick to the low end of the price range.

What do you expect to see at: "Requires full regression tests"? Your tests seem to not actually test for equality or anything or for the fact that some error must be raised, instead, it just checks if the regression-test-data slot runs without failure.

The generic "Lift" framework that we have in place already has utilities for seeding the regression-test-data, then running future tests to compare the regression-test-data with the previously seeded data. That part's not necessary to write for each individual test. So all that is necessary is to have test(s) which compute some useful regression-test-data.

[genworks]

Actually for this item I'm not sure how the regression-test-data approach would work. This one might need a manual test procedure. Actually two items here - the M-. and the reserved-word check for define-object-amendment.

[reiniervandijk]

Fix has been sent to you in a private message. Let me know if you have questions. Note that the test is automated, no need for a manual procedure. Note new test utilities that check for failure or absence of it. These are proposed as necessary add-ons to the lift framework and they allow for true automated testing.

Do you mean a pointer to the file or to the actual class object itself?

Pointer to the file(s)

Does this work for the case where multiple define-object-amendments are invoked?

yes

I would think it should present a list of all locations of definitions - similar to what you get for a defmethod which has several implementations for different argument signatures.

That's how it works.

Please forget that part of the assignment, and we'll stick to the low end of the price range.

Well, I've re-audited it for the extent of checking for reserved-symbol-overrides. Please, don't suddenly change the conditions for the bounty while someone is working on it.