On Wed, 2020-04-29 at 16:42 +0200, Jakub Jelinek wrote:
> On Wed, Apr 29, 2020 at 10:25:38AM -0400, David Malcolm wrote:
> > > I'd prefer not to at this point, all that could be commonized are
> > > the two inform calls perhaps with a bool or enum arg which one it
> > > is,
> > > but usually the backends prefer to keep their -Wpsabi stuff
> > > visible
> > > there.
> > > Yes, it affects 4 targets (s390 too; haven't touched that one,
> > > because
> > > there is a pending patch with two alternatives).
> >
> > One other benefit is to move the allocation/free of the URL string
> > so
> > that it's guarded by the same condition as the "inform" call.
> >
> > Is there a chance this patch could be doing a bunch of extra
> > allocation
> > and freeing of URL strings that never get used? How often does
> > this
> > code get called?
>
> I think it will be called infrequently, and emitting a diagnostic is
> already
> slow.
I was more concerned about the case for which the url is built but then
the "inform" is not called - I was worried it might happen per field of
a struct - but if you think that code path is infrequent, then I trust
your judgment.
> E.g. the URLs for warnings are also allocated and freed during the
> warning{,_at} call.
I believe I fixed that in abb4852434b3dee26301cc406472ac9450c621aa so
that it only builds the URLs if they're going to be used.
> So, not sure it is worth wasting a static variable on
> it, especially for all the other 52 or how many backends that don't
> need it.
>
> But if you feel strongly about it, can do it that way, though not
> really
> sure what is the best spot to place it, calls.[ch] ?
I don't feel strongly about it.
Thanks
Dave
