<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 21, 2016, at 11:53 AM, Teresa Johnson <<a href="mailto:tejohnson@google.com" class="">tejohnson@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Apr 21, 2016 at 11:44 AM, Duncan P. N. Exon Smith <span dir="ltr" class=""><<a href="mailto:dexonsmith@apple.com" target="_blank" class="">dexonsmith@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br class="">
> On 2016-Apr-21, at 11:41, Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:<br class="">
><br class="">
><br class="">
>> On Apr 21, 2016, at 11:25 AM, Teresa Johnson <<a href="mailto:tejohnson@google.com" class="">tejohnson@google.com</a>> wrote:<br class="">
>><br class="">
>> I am currently making some BitcodeWriter changes that involve some refactoring, and am thinking for the Nth time that it would be much nicer to have a class instead of passing around a long list of parameters. I am thinking of biting the bullet and doing that - any objections?<br class="">
><br class="">
> In general I'm worried about having single gigantic class that keep many data members, this goes against <a href="https://en.wikipedia.org/wiki/Single_responsibility_principle" rel="noreferrer" target="_blank" class="">https://en.wikipedia.org/wiki/Single_responsibility_principle</a> and makes it hard to track what is initialized, where, and under which condition (basically one of the reason why global variables are not welcome). The code is almost always easier to understand with small separated components (yes many places are drifting a lot in LLVM...).<br class="">
><br class="">
> Not to say that it can't be done, just that it requires a lot of care.<br class="">
<br class="">
</span>I don't think we should throw *all* the state in. But some of the state<br class="">
is passed everywhere.<br class="">
<br class="">
At least (off the top of my head):<br class="">
<br class="">
- ValueEnumerator&<br class="">
- BitstreamWriter&<br class=""></blockquote><div class=""><br class=""></div><div class="">Agree. Also the ModuleSummaryIndex, and the Module in the case of writing bitcode for a module as opposed to a combined index (which should probably be different derived classes). I'll try to be sensible, but you can let me know if it is too much when I have a patch. =)</div></div></div></div></div></blockquote><div><br class=""></div><div>The problem is that most of the time, patches in isolation are perfectly fine, it is only when you step back and see the result that you figure out you reach the state of <a href="https://raw.githubusercontent.com/llvm-mirror/llvm/master/lib/Target/X86/X86ISelLowering.cpp" class="">https://raw.githubusercontent.com/llvm-mirror/llvm/master/lib/Target/X86/X86ISelLowering.cpp</a> Multiple people adding stuff after you does not help to keep the overall vision as well.</div><div><br class=""></div><div>(To be clear: I don't mean I'm opposed to what you want to do)</div><div><br class=""></div><div>-- </div><div>Mehdi</div><div><br class=""></div></div></body></html>