Hi

2015-03-11 1:49 GMT-03:00 Stanislav Malyshev <smalys...@gmail.com>:

> Hi!
>
> > related to the proposed RFC. *But* after some heuristics it was
> > noticeable that most warnings had a common cause. I parsed the output
>
> It doesn't matter if it has common cause or not. If I have a system of
> Wordpress-like size, I'm bound to get a lot of failures, that's what it
> is telling me. And 27 separate failures are non-negligible number.
>
>
It's just 27 function calls to fix, and you only need to remove a few
arguments that are being ignored
because the functions are not taking them into account... I don't think it
has a big cost to fix.
I fixed them myself in less than one hour!

This won't be an E_INSANELY frequent warning and fix is straightforward, as
the warning
message is usually very explicit about where the call happens and where the
function was declared.


> > some code refactory that left function calls with residual parameters
> > behind. This could be fixed in less than 1 hour.
>
> I think you are severely underestimating the cost of fixing it. Fixing
> bugs is not only replacing the bytes.
>
>
Same as up there.


> > The ZF2 test suite couldn't run because of fatal errors caused by PHP7
> > incompatibilities
> > or bugs that caused segfaults. Nothing related to the RFC itself.
>
> We do not know if something there is related to RFC or not. That's not
> evidence that RFC is OK, that's evidence that a) we could not obtain the
> information and b) we already have a BC problem so severe that we can
> not run ZF2, and probably not easily fixable (since, I assume, if it
> were easily fixable you'd have done so).
>
That's why I am reluctant to
> add more BC breaks, especially ones that bring no new capabilities but
> just add more error messages. Each new BC break adds migration barriers,
> and in my opinion, it's not even linear.
>
>
I really get your concerns, but this RFC will not increase the migration
barrier
as much as you are thinking (not even close). Just look at the other
projects
that I tested too ;)

That's why I tried to measure the BC breaks, I was expecting more BC breaks
but it turned out surprisingly sparse breaks and most are potential bugs.

When you say "the RFC brings no new capabilities" I disagree with you. The
RFC
makes PHP safer and easier for people to catch problems, that's a good
capability.

> If compilation is "magic"... we are all magicians here. Calling the
> > implementation "magic"
> > won't change the fact that it's just a compilation check :D
>
> I didn't say compilation is "magic", you are strawmanning here. What I
> said is that having different behavior of the compiler depending on
> specific function calls made by the function being compiled is pretty
> rare (not only in PHP but in other languages too) and non-obvious, and
> thus suspicious.
>
>
The bad design is on PHP which unfortunately decided that variadic
functions should
be determined by func_get_args usage. I'm just dealing with that oddity by
doing a
compile time check. It's not "suspicious". It's predictable and reliable.

We check function signatures, we check return by ref and many other things.
If
you think it really straight there is nothing really shocking on the
proposed solution.
You might not like the RFC, but I don't think this is a point for concerns.


> > If you plan to use methods|functions interchangeably then you should
> > make them compatible
> > from now on. That's one of the points of the RFC. It's not "unclean".
>
> You basically just choose to ignore my argument here, recognizing that I
> correctly identified the case where BC break would happen but saying you
> don't care about it. Other people that do use such code may care.
> And yes, I consider adding unnecessary parameters to a function just to
> satisfy some warning "unclean".
>
>
I'll sincerely try to give some more thought on this subject. I don't think
it's 100% necessary
but I'll try to see if there is a possible alternative for this case.


> > BTW, the current PHP silent behavior should be considered even more
> > confusing otherwise we
> > wouldn't have these measurements:
> >
> > https://wiki.php.net/rfc/strict_argcount#bc_breaks_on_the_real_world
>
> I'm not sure how these prove current behavior is "confusing". Yes, there
> are bugs in the code, and I'm sure this feature will expose some of
> those (previously harmless) bugs.


"Harmless bugs" that one day might get promoted to heartbleed status
on some widely used package or CMS out there if you don't kill them ;)


> At cost of the BC break. And don't
> think a handful of libraries represent the vast universe of PHP code,
> most of which we can't even see because it's not published anywhere.
>

Unfortunately we can't run each in house code out there to prove something
or not,
but I had to test drive the patch and picked a diverse number of packages
and
FOSS projects so everybody can repeat the tests too.

There is no other way to prove usefulness and measure BCB other than
running the test
suites (it's all green) and running real projects that anyone can have
access to reproduce the tests.
And you also have the patch to test things yourself if you really have the
willpower.

If you think that the current methodology used for measurements is not
valid, I'm afraid there is
nothing else I can do to make it better.


> > It basically means that even experienced package maintainers are
> > shooting on their own foot and
> > they are using code evaluation tools, static analyzers, etc. Now imagine
> > the common PHP programmer.
>
> Yes, experienced programmers make bugs. Inexperienced programmers make
> bugs too. Nobody ever claimed otherwise. The point is not about that,
> the point is in this case the cure may be worse than the disease.
>
> > You can't claim that I artificially built the test suites of a lot of
> > open source
> > projects just to "support the RFC". The test suites (and the code being
>
> Coincidentally, I also didn't claim that. Should we go and enumerate
> more things that I can't claim and actually didn't, or we are ready to
> proceed to discussing things I actually did claim?
>
>
You said that the example given was "artificially built" to prove a point.
I just said
that I couldn't be artificially building the test suites from all the FOSS
projects tested
as sampling. And the example given was based on the experience of testing
these projects
and getting the results (it's far from "artificial").


> > - and there it is: the same patterns from the "artificial" example were
> > detected
> > numerous times "on the wild".
>
> No, your example talks about something completely different - removing a
> parameter used by the code and the tests completely missing that even
> though the API requires that parameter to be used, and then adding
> another parameter in place of that previous parameter and giving it
> completely different meaning and the tests missing it again. If some
> framework in the wild has such code changes and such tests, they need to
> seriously upgrade their testing game.
>
>
I don't want to sound repetitive, but that's exactly what's happening on
some of the tested code.
Parameters were removed and nothing warns about the wrong method|function
calls because PHP
happily swallow the arguments... until somebody create another argument
that will
suddenly take the dormant values being passed into account - bug.


> --
> Stas Malyshev
> smalys...@gmail.com
>

Márcio

Reply via email to