[llvm] r246367 - [Triple] Stop abusing a class to have only static methods and just use

[Chandler Carruth via llvm-commits]

On Sun, Aug 30, 2015 at 3:13 AM Renato Golin via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On 30 August 2015 at 03:09, Chandler Carruth via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: chandlerc
> > Date: Sat Aug 29 21:09:48 2015
> > New Revision: 246367
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=246367&view=rev
> > Log:
> > [Triple] Stop abusing a class to have only static methods and just use
> > the namespace that we are already using for the enums that are produced
> > by the parsing.
>
> Hi Chandler,
>
> So, about this one, I wish you had asked first.
>

This is a very simple change, and no one seemed actively modifying it, so I
don't think there is a problem here...


>
> This is a big mess, yes, but the answer is not that simple. It did
> occur to me to do it like that from the beginning, you'd be glad to
> know, but there are further design issues that stopped me from doing
> it, namely future inheritance when we get to a more generic target
> parsing.
>

I can't imagine how inheritance is any part of a correct design for a text
parsing solution here.

Even if there are some future plans, we shouldn't design the code in a way
that makes no sense today because we think (without evidence) that it will
make more sense in the future. YAGNI, or insert your software design
aphorism here.

This code, today, is doing procedural text parsing, and as such should just
be using functions in a name space. If and when some future design is
actually proposed, I think that would be the time to change it.


>
> Could you please revert your changes on TargetParser until we can all
> agree on an implementation that makes sense to the future and present?
>

I don't think that is the correct approach. This patch in particular I feel
*extremely* confident is a strictly positive step forward.

We should not revert to a worse state while we figure out some hypothetical
better state.

Now if you want to remove *all* of this code until that better state is
identified, that makes more sense but I think that ship has sailed. That
would be perceived as an acceptable regression. The only path forward I see
here is to make the code that we have today be reasonable, and ask you and
others to try to come up with a better design long term. I think these
patches are reasonable steps in the first goal.


>
> cheers,
> --renato
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150830/9e7b3e9e/attachment.html>