Patch forthcoming.
Created attachment 238190 [details] the patch
Attachment 238190 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:139: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:146: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:152: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:158: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 4 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #2) > Attachment 238190 [details] did not pass style-queue: > > > ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:139: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] > ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:146: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] > ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:152: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] > ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:158: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] > Total errors found: 4 in 4 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. I fixed all of these locally.
Comment on attachment 238190 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=238190&action=review r=me with the reference change > Source/JavaScriptCore/ChangeLog:19 > + This reduces the amount of style points one could conceivably get in the future when > + hacking JSC, but creating a single reusable thingy for computing local OSR availability. The last half of this sentence doesn't seem to make sense - early finish? > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6747 > + Operands<Availability> availability() { return m_availabilityCalculator.m_availability; } > + const Operands<Availability>&
(In reply to comment #4) > (From update of attachment 238190 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=238190&action=review > > r=me with the reference change > > > Source/JavaScriptCore/ChangeLog:19 > > + This reduces the amount of style points one could conceivably get in the future when > > + hacking JSC, but creating a single reusable thingy for computing local OSR availability. > > The last half of this sentence doesn't seem to make sense - early finish? s/but/by I will fix. > > > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6747 > > + Operands<Availability> availability() { return m_availabilityCalculator.m_availability; } > > + > > const Operands<Availability>& Why? What does it buy us? I don't like adding const unless I have a story for what it will accomplish, since pointless uses of const run the risk of just requiring more code to be written (const/non-const variants of things...). In this case, you can already edit the availability by doing m_availabilityCalculator.m_availability. A humorous example of when you last asked me to make something const was Node::child1()/child2()/child3(). The result was that everyone would wanted to edit children had to be more verbose - Node->children.setChild1(...). This wasn't a good thing. I take it as a given that the purpose of a compiler's internal data structures is that they may be edited. There is usually no point in dragging const around.
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 238190 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=238190&action=review > > > > r=me with the reference change > > > > > Source/JavaScriptCore/ChangeLog:19 > > > + This reduces the amount of style points one could conceivably get in the future when > > > + hacking JSC, but creating a single reusable thingy for computing local OSR availability. > > > > The last half of this sentence doesn't seem to make sense - early finish? > > s/but/by > > I will fix. > > > > > > Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp:6747 > > > + Operands<Availability> availability() { return m_availabilityCalculator.m_availability; } > > > + > > > > const Operands<Availability>& > > Why? What does it buy us? > > I don't like adding const unless I have a story for what it will accomplish, since pointless uses of const run the risk of just requiring more code to be written (const/non-const variants of things...). In this case, you can already edit the availability by doing m_availabilityCalculator.m_availability. > > A humorous example of when you last asked me to make something const was Node::child1()/child2()/child3(). The result was that everyone would wanted to edit children had to be more verbose - Node->children.setChild1(...). This wasn't a good thing. > > I take it as a given that the purpose of a compiler's internal data structures is that they may be edited. There is usually no point in dragging const around. *facepalm* Discussion on irc revealed that I was using Operands<Availability> without a &. Ollie recommends Operands<Availability>& (i.e. non-const ref). I will do that.
Landed in http://trac.webkit.org/changeset/173672