[llvm-dev] Refactor BitcodeWriter into classes?

[Teresa Johnson via llvm-dev]

On Thu, Apr 21, 2016 at 11:44 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2016-Apr-21, at 11:41, Mehdi Amini <mehdi.amini at apple.com> wrote:
> >
> >
> >> On Apr 21, 2016, at 11:25 AM, Teresa Johnson <tejohnson at google.com>
> wrote:
> >>
> >> 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?
> >
> > In general I'm worried about having single gigantic class that keep many
> data members, this goes against
> https://en.wikipedia.org/wiki/Single_responsibility_principle 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...).
> >
> > Not to say that it can't be done, just that it requires a lot of care.
>
> I don't think we should throw *all* the state in.  But some of the state
> is passed everywhere.
>
> At least (off the top of my head):
>
>   - ValueEnumerator&
>   - BitstreamWriter&
>

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. =)

Good idea on lower-casing function names at the same time, at least for
anything that moves into the class.

Thanks,
Teresa



-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160421/2fdcee09/attachment.html>