Otherwise, an Availability may point to a node that was removed from the graph.
(In reply to Saam Barati from comment #0) > Otherwise, an Availability may point to a node that was removed from the > graph. Or just not report availability in dumping. I don't think it's a good idea to have dumping cause us to run an analysis. We want to be able to dump when debugging. If there's a bug then computing an analysis may trigger another bug.
<rdar://problem/39481690>
(In reply to Filip Pizlo from comment #1) > (In reply to Saam Barati from comment #0) > > Otherwise, an Availability may point to a node that was removed from the > > graph. > > Or just not report availability in dumping. > > I don't think it's a good idea to have dumping cause us to run an analysis. > We want to be able to dump when debugging. If there's a bug then computing > an analysis may trigger another bug. Yeah I'm torn on this. I personally find dumping availability helpful. But I also thought through an issue you're describing here. Making an analysis run in dumping could make other analyses break in weird ways when they may be using the availability data structures. I think we should just remove the dumping, and if someone needs that data dumped, they can manually hack the dumping code.
Created attachment 349801 [details] patch
Comment on attachment 349801 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=349801&action=review r=me > Source/JavaScriptCore/dfg/DFGGraph.cpp:572 > + if (false) Can you make this a static const bool at the top of this file? It makes it easy to enable by just changing the bool.
Created attachment 349802 [details] patch for landing Thanks for the review
Comment on attachment 349802 [details] patch for landing Clearing flags on attachment: 349802 Committed r236022: <https://trac.webkit.org/changeset/236022>
All reviewed patches have been landed. Closing bug.