[llvm-commits] [llvm] r107667 - /llvm/trunk/include/llvm/Instructions.h

[Gabor Greif]

On 6 Jul., 17:56, Chris Lattner <clatt... at apple.com> wrote:
> On Jul 6, 2010, at 8:44 AM, Gabor Greif wrote:
>
> > Author: ggreif
> > Date: Tue Jul  6 10:44:11 2010
> > New Revision: 107667
>
> > URL:http://llvm.org/viewvc/llvm-project?rev=107667&view=rev
> > Log:
> > second round of low-level interface squeeze-out:
> > making all of CallInst's low-level operand accessors
> > private
>
> > If you get compile errors I strongly urge you to
> > update your code.
>
> Hi Gabor,
>
> I'm all for the spirit of this patch, however, this is really really gross:
>
>
>
> > +# define public private
> > +# define protected private
> >   /// Provide fast operand accessors
> >   DECLARE_TRANSPARENT_OPERAND_ACCESSORS(Value);
> > +# undef public
> > +# undef protected
>
> How about declaring a different macro, named DECLARE_PRIVATE_TRANSPARENT_OPERAND_ACCESSORS or something?
>

Heh, here is my answer from IRC:

 gabor: sabre: re. gross, yes, but I wanted to minimize the patch.
Actually, my first attempt did just what
 you suggest, but it touched OperandTraits.h, and that appeared too
much to me. This hack is strictly
 transitional, but if we think it should be in v2.8 (and be removed
from v2.9) then I am all for rephrasing
 in a less gross way :-)


Classical case of most bang for the buck...

...but it basically boils down to:

1) remove this hack before 2.8 branches and warn external
   users by release notes
2) keep in 2.8 release and remove from trunk then
3) keep well into the future.

I am hesitant with 1) as people often only track the changes when
releases come around. 3) seems to be overly rude by breaking the
Liskov substitutionality (do not take away what you have promised in
the baseclass). So I am all for 2). This means I'll reformulate as
soon as my actual patch has finally landed and stuck. Ok?

Cheers,

    Gabor

> -Chris
>
> _______________________________________________
> llvm-commits mailing list
> llvm-comm... at cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits