Make third argument of DeclareRepresentation and NewRepresentation optional, and document that it and the fourth argument are (and always were) unused
#4519
Conversation
fingolfin
force-pushed
the
mh/DeclareRepresentation-args
branch
3 times, most recently
from
be720cb
to
8bff08d
Compare
There was a problem hiding this comment.
I agree with the changes, and have made just one comment concerning a formulation.
Concerning the newly inserted # TODO comments:
I think they might be confusing for readers in the future.
The filters in question have not been used before, in particular they were not intended to be automatically implied by the newly created filters.
lib/type.g
Outdated
| ## The admissible positions resp. component names of <A>super</A> need not | ||
| ## be listed in <A>slots</A>. | ||
| ## The optional third and fourth arguments <A>slots</A> and <A>req</A> | ||
| ## are unused (and never were) only provided for backwards compatibility. |
There was a problem hiding this comment.
perhaps "are (and always were) unused and only provided for backwards compatibility"
There was a problem hiding this comment.
Indeed, that's much better. Fixed now
fingolfin
force-pushed
the
mh/DeclareRepresentation-args
branch
from
8bff08d
to
8a5297d
Compare
fingolfin
changed the title
DeclareRepresentation and NewRepresentation optional, and document that it and the fourth argument are (and always were) unused
fingolfin
added
the
release notes: use title
fingolfin
changed the title
DeclareRepresentation and NewRepresentation optional, and document that it and the fourth argument are (and always were) unusedDeclareRepresentation and NewRepresentation optional, and document that it and the fourth argument are (and always were) unused
- document that their 3rd and 4th arguments are unused - make the 3rd argument optional (the 4th already is)
... by printing a trailing newline, just like `Info` does
fingolfin
force-pushed
the
mh/DeclareRepresentation-args
branch
from
8a5297d
to
d18d279
Compare
|
The reason I added those TODO comments is because without them, it feels as if some information is lost... and yeah, I know these were not implied, but I still wonder why? I guess this is the same underlying issue as in PR #4499: Why doesn't On the other hand, we have had plenty of ranking issues (esp. with "hidden implications" disabled) because people used (and use) representations in method argument filters, unaware that these filters lack many seemingly "obvious" implications. Anyway, I don't think this particular instance is worth arguing about a lot, so I just dropped these comments now shrug |
fingolfin
added
the
kind: removal or deprecation
See the commit messages for details.
See also issues #4409 and #1042