<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 21, 2015 at 7:27 PM, Pete Cooper <span dir="ltr"><<a href="mailto:peter_cooper@apple.com" target="_blank">peter_cooper@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">This is more of a general question, but your patch is a good example (This shouldn’t block committing BTW).<div><br></div><div>You’ve passed in InstCombiner &IC but only ever use it for IC.Builder.  Do you have a feel for when we should pass in a variable such as IC vs just pass in the bit we want like IC.Builder?  Makes the caller slightly more complex, but shows the scope of what we’re actually using.</div></div></blockquote><div><br></div><div>I totally think this is an anti-pattern. I'm a bit embarressed to be spreading it, but oh well.</div><div><br></div><div>So, my principled answer would be: we should definitely be passing the builder, or we should be making a private method of InstCombiner.</div><div><br></div><div>In this specific case, I wasn't planning to only use the builder. And indeed I'm about to submit a patch that uses DataLayout as well. What I actually wanted was a private method on InstCombiner. I think that's what roughly everyone wants here. Unfortunately, adding a private method means going up to the header and adding another line, blah blah blah. I think that's why this pattern pervades. That doesn't make it good, and we should try to develop some reasonable practices that avoid it. I just don't have those yet. I'll think more about what I think the right patterns would be, as there are actually a *lot* of choices.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div>This is perhaps more interesting in the backend where I think i’ve seen a bunch of functions take a MachineFunction when they just needed something like MachineRegisterInfo from the MF.</div></div></blockquote></div><br>Agreed. I see it all over the place.</div><div class="gmail_extra"><br></div><div class="gmail_extra">The worst offender is Sema in Clang. It is the most egregious and amazing example of this pattern I have ever seen.</div></div>