Dan Ackroyd wrote: >> The developer who made that change would run the tests, >> see the tests pass and then push their code live. At which point, it >> would cause an error in production, as the string returned would not >> pass the is_literal check.
Craig Francis wrote: > No, the only way fatal errors can occur is if you choose to make them fatal > (in userland code). Please can you go into some detail about what you think people are meant to do when they detect a non-literal used where a literal is expected? This seems like a core flaw in the proposed RFC and I think you aren't addressing it. > that's going to make adoption very difficult. Why are you prioritising speed of adoption, over reducing the cost of using this feature for projects over say the next 5 or 10 years? It kind of sounds like the idea behind the RFC has been a compromise between the idea in Mr Kern's talk, and people who are not sure this feature is a good idea or not. This feature should be of most use to larger teams, where reasoning about the application is difficult to begin with. For them an extra few hours migrating a large code base to use the appropriate techniques is a fine trade-off over increased difficulty maintaining their code. > No, the only way fatal errors can occur is if you choose to make them fatal > (in userland code). If people aren't going to make using a non-literal where a literal is expected, be an error, the only alternative I can see is logging it. Please correct me if you think people should be doing something other than those two things. >From my experience, any process that involves looking in log files is a pretty bad process. For is_literal checks, what's going to happen is: * programmer makes a mistake, that isn't caught by a test. * programmer deploys site, and entries about non-literal strings start being written to log files. * someone then needs to remember to check the log files, and pull out the appropriate entries, and make a ticket for them to be investigated. * someone then needs to pick up that ticket, investigate the cause of the non-literal string being used, and trace it back to which module/library is causing it, then update the ticket for someone on that team to fix it. * someone on that team needs to have that ticket prioritised before they start work on it. Apologising for expressing myself poorly before, but this is a really, really, really, really, really, really result to have. It makes using this feature either have a high maintenance burden, and allows security flaws to exist until someone gets around to fix them, or it results in things failing in production. I know it's slightly annoying to require any combining of strings, where you want to preserve 'literal-ness', to have to jump through the hoop of using a specific function, but it's just a few seconds work, that would save hours of debugging. The whole reason why the solution worked for Google was that it made it cost less for programmers to do the right thing, rather than having to remember fix problems after they are found.* I don't think there would be an easy way to fix this if the RFC was passed in its current form. Changing from string concatenation carrying the literal flag around to not, would be a huge BC break, that would require multiple versions to fix. If it was the other way round so that we don't support carrying the literal flag around, and (much to my embarrassment) it does mean that people can't actually use the feature, because it's too difficult, it would be much easier to move to carrying the flag around. > However requiring developers to rewrite all of their code to use > literal_concat() and literal_implode() No-one is forcing developers to rewrite their code. And I don't think many people would actually use those functions. The vast majority of people are going to continue to use the functions provided by Wordpress, Doctrine etc, and any checks on literal, or combining of literals would be internal to those functions. As I wrote to Thomas, I think the majority of people are more likely to shift to using type specific wrapping types, rather than copying bare strings around. cheers Dan Ack *For anyone wondering, unfortunately we can't use the same implementation as Google, due to PHP not being statically compiled, and also not being able to set 'type flags' on strings. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
