Unspecified and unused optional argument req in NewRepresentation and DeclareRepresentation

[fingolfin]

The documentation for NewRepresentation and DeclareRepresentation mentions and optional fourth argument req but does not specify what it does.

Looking at the implementation, as far as I can tell it is simply ignored:

BIND_GLOBAL( "NewRepresentation", function ( arg )
    local   rep, filt;
...
    # Create the filter.
    if LEN_LIST(arg) = 3  then
        rep := NEW_FILTER( arg[1] );
    elif LEN_LIST(arg) = 4  then
        rep := NEW_FILTER( arg[1] );
    else
        Error("usage: NewRepresentation( <name>, <super>, <slots> [, <req> ] )");
    fi;
...

I looked back through our extended version history, and it was like that since the initial import of GAP4 into CVS on 1996-09-25. So maybe this is code that simply was never finished

Yet some code "uses" it:

DeclareRepresentation(
    "IsGF2VectorRep",
    IsDataObjectRep and IsVectorObj, [],
    IsRowVector );

Unfortunately it is not completely clear to me what the intention is -- how does this optional list of filters differ from the second argument super?

Maybe NewFamily gives a hint, which has argument req (for "required") and imp (for "implied"), which are also both filter lists. But the first thing it then does with them is to join them together:

    imp_filter := WITH_IMPS_FLAGS( AND_FLAGS( imp_filter, req_filter ) );

Well, that and store req into family!.REQ_FLAGS and use that for printing...

We should figure out which code uses this unused 4th argument (e.g. by removing it, then trying to load GAP and packages, and see what breaks); and then decide how to proceed. E.g. if not much uses it, we could just remove it and "fix" all affected call sites.

[ThomasBreuer]

The documentation of NewRepresentation and DeclareRepresentation mentions the optional fourth argument but does not state what it means. The code ignores the fourth argument, thus it cannot be accessed from GAP's internal data. Thus I think we should first remove it from the documentation, and change the places in the GAP library where a fourth argument is given.

(I do not remember what the intended meaning of <req> was.)

The usual way to deal with obsolete functionality (or here rather obsolete "functionality") is to keep the old stuff in lib/obsolete.gi. We could provide three-argument versions of NewRepresentation and DeclareRepresentation in the library and then replace them by vararg versions in lib/obsolete.gi, but this is perhaps not a good idea.

By the way, also the third argument slots is not used anywhere in the code, and an empty list is entered in most calls, due to the fact that GAP does not take it seriously. We could remove also this argument, or at least make it optional (and then ignore the value).

[fingolfin]

Ugh, I just discovered that I reported this before, as #1042... sigh. I'll leave both open for now.

That issue point out that also the third argument is unused! But it is not optional. I suggest that we make it optional. In the documentation, I would mention that those arguments used to be around but were unused (so that people who compare existing code to the documentation are not confused by seeing variants with 2 / 3 / 4 arguments).

Then change the GAP library to not require or mention those arguments

We could also modify the code in NewRepresentation by adding a Info( InfoObsolete, "..." ); warning if use of the fourth argument is detected and the obsolete level is high enough (normally it won't be).

Finally, if obsoletes are disabled (which we can detect by checking for not IsBound(GAPInfo.Read_obsolete_gd), or by adding a nicer helper function IsObsoleteCodeDisabled, but that might be overkill?), we could issue an error instead of a warning.

[ThomasBreuer]

This can be closed since #4519 has been merged.