gap icon indicating copy to clipboard operation
gap copied to clipboard

Objectify: warn if Is{Component,Positional}ObjectRep absent

Open fingolfin opened this issue 2 years ago • 1 comments

This works towards resolving #1043: now Objectify warns if IsPositionalObjectRep resp. IsComponentObjectRep is not set for an object that should have one of them; and it also gives an error if both are set / "the wrong one" is set (i.e. IsPositionalObjectRep for a T_COMOBJ or IsComponentObjectRep for a T_POSOBJ).

I've eliminated all the warnings triggered by loading GAP and running tst/testinstall.g, and submitted https://github.com/gap-packages/fr/pull/50 to fix one violation in a package (I've already fixed a bunch of others in packages in the past couple years). I am guessing there may still be a couple more, so I am reluctant to turn the warnings into an error just now... at the very least, before we attempt such a thing, the test suites of all packages should be run against a GAP with that error activated.

Also, before merging this PR we should check how this extra test affects performance: Objectify is a bit of a bottleneck, and we don't want to make it worse than it already is...

fingolfin avatar Jul 29 '22 21:07 fingolfin

Hmm, thinking about it, I am not even sure why to warn -- instead we could just add that flag? Perhaps for performance reasons: a missing flag that has to be added in requires us to create a new sub-type...

fingolfin avatar Dec 19 '22 21:12 fingolfin