----- Mail original ----- > De: "Claes Redestad" <claes.redes...@oracle.com> > À: "Remi Forax" <fo...@univ-mlv.fr> > Cc: "core-libs-dev" <core-libs-dev@openjdk.java.net> > Envoyé: Vendredi 29 Mai 2020 18:47:03 > Objet: Re: RFR: 8246152: Improve String concat bootstrapping
> Hi Rémi, > > thanks for looking at this and suggesting some improvements! > > On 2020-05-29 17:51, Remi Forax wrote: >> Hi Claes, >> For the code below the comment "Mock the recipe to reuse the concat generator >> code", >> i believe you can use String.repeat() that was introduced recently. > > Sure, > >> >> The code that parse the the receipe can be in its own method to make the code >> more readable, >> this method returns the list and use the StringBuilder internally. > > sure, > >> >> In generateMHInlineCopy, >> element.get(0) and element.get(1) should be stored in local variables >> after >> "elements.size() == 2" >> will make the code more readable > > sure, > >> >> In simpleConcat(), >> should use a local variable 'mh', like the newString or newArrayWithSuffix > > yes! > > http://cr.openjdk.java.net/~redestad/8246152/open.01/ I don't think you need to check MAX_INDY_CONCAT_ARG_SLOTS anymore, given that now indy is based on invokeWithArguments instead of invoke. And i think you should keep the null checks of constants upfront to not change the semantics (you are allowing constant not referenced by a recipe to be null). The catch should be: catch (Error | RuntimeException e) { if you want the StringConcatException to be propagated all the other changes looks good. > > Re-running tier1 > > /Claes Rémi
