GNU bug report logs - #17381
_GL_ATTRIBUTE_PURE in dfa.h

Previous Next

To reply to this bug, email your comments to 17381 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: bug-grep <at> gnu.org
Subject: _GL_ATTRIBUTE_PURE in dfa.h
Date: Thu, 01 May 2014 00:19:40 -0600

This breaks the gawk build. I have to copy the defines for it from dfa.c
into dfa.h to make it compile.

I haven't looked hard. Are all the new routines in dfa.h needed by
code in grep? If not, could they be made static in dfa.c?

Can we try in the future to the gnulib-specific stuff out of the
header?

Thanks,

Arnold

Message #8 received at 17381 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: arnold <at> skeeve.com, 17381 <at> debbugs.gnu.org
Subject: Re: bug#17381: _GL_ATTRIBUTE_PURE in dfa.h
Date: Wed, 30 Apr 2014 23:33:59 -0700

arnold <at> skeeve.com wrote:
> I have to copy the defines for it from dfa.c
> into dfa.h to make it compile.

It'd be better to move that stuff into custom.h, I expect.  Please see 
below.

> Are all the new routines in dfa.h needed by code in grep?

Yes, I'm afraid so.  We wouldn't have put them there otherwise.

> Can we try in the future to the gnulib-specific stuff out of the
> header?

In the long run it'd be better to have the header just work, so that you 
don't have to copy anything from dfa.c to dfa.h or vice versa, and so 
that you can just use grep's dfa.c and dfa.h with little or no change.

I emailed you something along those lines earlier this month; you can 
find a copy at <http://bugs.gnu.org/17157#61>.  If you'd like me to come 
up with a revised patch, which merges this into the latest gawk master, 
I can do that.

Message #11 received at 17381 <at> debbugs.gnu.org (full text, mbox):

From: Aharon Robbins <arnold <at> skeeve.com>
To: eggert <at> cs.ucla.edu, arnold <at> skeeve.com, 17381 <at> debbugs.gnu.org
Subject: Re: bug#17381: _GL_ATTRIBUTE_PURE in dfa.h
Date: Thu, 01 May 2014 21:29:43 +0300

Hi Paul.

> > I have to copy the defines for it from dfa.c
> > into dfa.h to make it compile.
>
> It'd be better to move that stuff into custom.h, I expect.  Please see 
> below.

custom.h is for system customization to override things that Autoconf
can't figure out or gets wrong.

> > Are all the new routines in dfa.h needed by code in grep?
>
> Yes, I'm afraid so.  We wouldn't have put them there otherwise.
>
> > Can we try in the future to the gnulib-specific stuff out of the
> > header?
>
> In the long run it'd be better to have the header just work, so that you 
> don't have to copy anything from dfa.c to dfa.h or vice versa, and so 
> that you can just use grep's dfa.c and dfa.h with little or no change.

Requiring gnulib in that header makes it less attractive to other
projects that might want to use dfa as a black box.

Are there such?  I don't know. (I thought I'd heard something about
gettext using dfa but I am unsure if that is true.)

It is certainly making things more painful for me.

Does the GL_PURE stuff have to be on every declaration? Or can it
just be on the body?  What does it even mean, anyway?  Does whatever
optimization it enables *really* make a big difference, or is it just
a micro-optimization?  (Serious questions.)

> I emailed you something along those lines earlier this month; you can 
> find a copy at <http://bugs.gnu.org/17157#61>.  If you'd like me to come 
> up with a revised patch, which merges this into the latest gawk master, 
> I can do that.

Yes, I know. I am unsure if your patch, which totally eliminates the
ability to compile gawk on systems without multibyte support, is the
direction in which I wish to go.  It may be, but I haven't done the
research yet to be sure that I won't break any of the systems I support.

I just looked at the patch again.  It really doesn't do the trick;
there are lots of places where MBS_SUPPORT is checked in the gawk code
and pulling mbsupport.h out of awk.h is likely to break things.

Thanks,

Arnold

Message #14 received at 17381 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, 17381 <at> debbugs.gnu.org
Cc: 17072 <at> debbugs.gnu.org, 17157 <at> debbugs.gnu.org
Subject: Re: bug#17381: _GL_ATTRIBUTE_PURE in dfa.h
Date: Thu, 01 May 2014 23:11:06 -0700

[Message part 1 (text/plain, inline)]

On 05/01/2014 11:29 AM, Aharon Robbins wrote:
> custom.h is for system customization to override things that Autoconf 
> can't figure out or gets wrong

OK, it's easy to have something else include mbsupport.h instead. 
config.h, say.  The attached patch does that.  It doesn't really matter 
what includes it, so long as it's done before dfa.c and dfa.h start 
using the multibyte functions.

> Requiring gnulib in that header makes it less attractive to other 
> projects that might want to use dfa as a black box. Are there such? I 
> don't know. (I thought I'd heard something about gettext using dfa but 
> I am unsure if that is true.)

gettext uses gnulib, so that's not an issue.

> Does the GL_PURE stuff have to be on every declaration? Or can it just 
> be on the body?

It should be on the declaration for external functions, so that the 
function's caller knows to optimize it.

> What does it even mean

It means the function has no effects except the return value and that 
the return value depends only on the parameters and/or global variables.

> Does whatever optimization it enables *really* make a big difference, 
> or is it just a micro-optimization?

We put it in because GCC nowadays complains if we leave it out, if we 
configure with --enable-gcc-warnings.  The optimization seems to be a 
win in general and (more important) an aid for humans reading the code, 
so we typically just add the pure attribute and move on.

> Yes, I know. I am unsure if your patch, which totally eliminates the 
> ability to compile gawk on systems without multibyte support

It's not supposed to do that.  It's supposed to work on those hosts, by 
supplying substitutes for wchar_t, wctype_t, etc.  Hmm, are you worried 
about hosts that don't even have wchar.h and wctype.h?  If so, that can 
be worked around reasonably easily; please see attached patch.

> I just looked at the patch again. It really doesn't do the trick; 
> there are lots of places where MBS_SUPPORT is checked in the gawk code 
> and pulling mbsupport.h out of awk.h is likely to break things

No, it should still work.  With the revised patch, config.h includes 
mbsupport.h, so MBS_SUPPORT will be defined appropriately for gawk code 
and gawk's other MBS_SUPPORT usages will continue to work as before.

I'll CC: this to Bug#17157 and Bug#17072 as it's following up to the 
last messages in both those threads, too.

[0001-awk-simplify-dfa.c-by-having-it-not-include-mbsuppor.patch (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.