develooper Front page | perl.perl5.porters | Postings from January 2017

[perl #130591] [PATCH] Make arguments to S_invlist_is_iterating andS_invlist_max be const

Thread Next
Karl Williamson via RT
January 26, 2017 05:03
[perl #130591] [PATCH] Make arguments to S_invlist_is_iterating andS_invlist_max be const
Message ID:
On Sat, 21 Jan 2017 01:11:24 -0800, hv wrote:
> On Fri, 20 Jan 2017 07:02:54 -0800, petdance wrote:
> > > On Jan 20, 2017, at 3:29 AM, wrote:
> > > Why the additional change here, was it somehow required for the
> > > consting?
> >
> > Yes.
> Thanks, but I had hoped for a more engaged response.
> I had also written:
> > It looks like care had been taken to abstract all references to
> > iterator
> > behind get_invlist_iter_addr(), this would violate that abstraction.
> I understand and value the benefits of abstraction and encapsulation
> in a complex codebase, and find them all too rarely in perl's. I'm
> much vaguer on the benefits of consting - they seem pretty minor, and
> regularly outweighed by bugs introduced - so my default preference
> would be to retain the former and reject this patch.
> However it might also be possible to provide the benefits of both, eg
> by splitting the r/w accessor into separate rvalue and lvalue
> variants.
> Karl, I think you're the one most involved with this code, do you have
> any opinion?

I strongly prefer keeping the encapsulation.

As many others have said (I'm guessing), I wish this had been written in C++, and then people wouldn't even be tempted to break it.

The underlying implementation of inversion lists was changed once already, and the effects were limited because of the encapsulation.  We may change it again, so that it isn't an SV type, and this would break at that point.

My position on consting is that I try to use it, but if it leads to any compile issues, I immediately abandon it.
> Hugo

Karl Williamson

via perlbug:  queue: perl5 status: open

Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About