Re: bison-20001221

[Hans Aberg]

At 14:17 +0100 1-01-12, Akim Demaille wrote:
>> - On a number of places there is include <config.h> It should be
>> include "config.h"
>
>I never quite understood the difference between the two, and some
>people will tell you to do just the opposite (see the Autoconf doc for
>a start).

#include <...> searches on the library directories, whereas
#include "..." first searches the local directories and then the library
ones -- or so I think is the normal way.

>Hans> - Files "files.c" and "complain.c" reads the macro definition
>Hans> differently: "files.c" somehow gets it from "error.h", I think,
>Hans> whereas "complain.c" does not read it at all. So I had to throw
>Hans> in #ifndef __attribute__ #define __attribute__(x) #endif into
>Hans> the file "complain.h" in order to be sure is always defined
>Hans> (which should be empty on my system).
>
>Err?  What macro definition?

I do not know. As I do not have access to "diff", I will try sending you a
tgz that you can run diff on.

>Hans> - In files "output.c", output_headers(), and "print.c",
>Hans> print_grammar(), I got problems with the fact that pre-MacOS X
>Hans> only allows stack allocations <= 32 kB, here exceeded by the
>Hans> "obstack_..." macros.
>
>Too bad...  Seriously, I won't address this issue.  If you want to do
>something, fix obstack and send the patches to the Glibc guys.  Be
>ready to fight if you really want to have MacOS stuff included in
>there :)

I just report this kind of stuff. I recall I simply made your allocations
static, andit worked with no problem.

Another idea I had is to make obstack into a FILE -- simply double
allocations when space runs out, and total copy time becomes linear. But I
did not get to it -- and I usually program in C++.

>Hans> This just something platform specific of an old OS, but I think
>Hans> in general, it is wise to not allocate more on the stack than
>Hans> needed. (For example, each obstack_fgrow1 asks for a stack
>Hans> allocation of 4 kB, whereas in output_headers() the actual
>Hans> allocation needed for each obstack_fgrow1 is less than 64 B.)
>
>I'm really surprised.  What makes you think that the stack is
>involved?  Aren't you referring to the heap instead?  Obstack is
>no more aggressive with the stack than malloc is.

It is not obstack causing the problems but your "system.h"
  #define obstack_fgrow1(Obs, Format, Arg1)     \
  do {                                          \
    char buf[4096];                             \
    sprintf (buf, Format, Arg1);                        \
    obstack_grow (Obs, buf, strlen (buf));      \
  } while (0)
specifically your
  char buf[4096];
-- I merely changed this to
  static char buf[4096];
and ther were no problems (but this isn't thread safe anymore).

>Hans> - I got around the problem by changing the obstack_fgrow1 macro
>Hans> containing a static array: static char buf[4096]; Not pure
>Hans> anymore, but it does not make any difference in my port.
>
>Aaaaah!  So you are talking about _my_ macros (see system.h).  Well,
>your compiler is dumb: at no time several buffers of 4096 are
>concurrently used.  Complain to the author of the compiler explaining
>that objects with non overlapping lifetimes can share the same storage.

OK.

>Hans> - Files "quote.c", "quote_arg.c" includes platform specific
>Hans> <sys/types.h>.
>
>So what?

Causes compiler error.

>Hans> Unnecessary if "quotearg.h" includes <size_t.h>.
>
>???  No it does not.

Well, I made some such changes and it worked on my system.

>Hans> Also, #include <quotearg.h> #include <quote.h> only works if one
>Hans> puts the stuff in the library; not if one takes down Bison and
>Hans> compiles it directly; #include "quotearg.h" #include "quote.h"
>Hans> is safer.
>
>I don't understand what issue you address here.  It compiles fine in
>here, and that's all I'm asking for.  I'm importing these files from
>elsewhere and will not make them Bison specific if that's what you mean.

The guys writing those libraries you stole seems to have a penchant for
#include <..> instead of #include "...". This means that when you steal
them and pack them in a distribution which is not going to be installed
where the standard <...> headers are, one needs to change them to "...". :-)

>Hans> - File "quote_arg.c", "else" clause of "#if HAVE_MBRTOWC": macro
>Hans> "iswprint" is already in the C-standard, so redefining it here
>Hans> causes an error.
>
>This is good to know.  Please let me see the error.

I don't know what you mean here.

>Hans> - File "bison.simple", int yyparse (YYPARSE_PARAM_ARG)
>Hans> YYPARSE_PARAM_DECL does not have a prototype -- produces a
>Hans> compiler warning for example when compiling under C++.
>
>Yeah, but for the time being I don't care.  Sorry about this, I truly
>am, but we have to set priorities.

If you finish this version, we can start thinking about C++ later.

  Hans Aberg