Re: cpplib: rewrite of include file handling, take two

[Zack Weinberg <zack at rabi dot columbia dot edu>]

On Tue, 24 Nov 1998 11:02:57 -0500, Dave Brolley wrote:
>
>
>Zack Weinberg wrote:
>
>> This patch fixes a large number of bugs and crocks in the previous
>> version.  I can now bootstrap the compiler with all languages enabled
>> and --enable-cpplib.  I will be running a glibc compile tonight, but I
>> think it will work, so I'm sending this out now.
>
>Let us know how that goes.

Completed successfully modulo the usual problems with snapshots and
glibc (it miscompiles some stuff; this is a known problem and has
nothing to do with cpp).

>> - gcc.dg/cpp-mi.c:  The new redundant include handling scheme
>> doesn't work for "" includes found in the current directory.
>> Run this file through the preprocessor once with no special options,
>> and once with `-I. -I-'.  You'll see that it works in the second case,
>> and not the first.  I'm open to suggestions how to fix it - it's not
>> easy.
>
>Can you outline the underlyng problem and why it's going to be hard to fix?
	
The new include searching works like this:  There's a hash
table indexed by the `short name' of the file (whatever the user put
on the #include line).  The hash entries have `foundhere' pointers
that point into the chain of directories on the include search path.
Since you can have several different #include files with the same
short name (by use of #include_next, or "" includes in different
directories, or "" and <> includes with different search paths)
there's a chain of all the files with the same short name
(->next_this_file in struct include_hash).

redundant_include_p is passed a hash entry for some short name, and a
place in the include path to begin looking.  If it finds an entry on
the include path whose address is equal to the ->foundhere pointer of
the hash entry, then the file to be included is the same as the
(previously included) hash entry, and ->control_macro in that hash
entry should be examined to see if the include is redundant.

"" includes search the `current directory' when -I- is not in effect.
The `current directory' is the directory where the top file on the
buffer stack lives.  I implement that by allocating a search path
entry on the fly for each call to do_include and tacking it on the
front of the chain.  This means that the address of the `current
directory' entry is different for each call to do_include, so
redundant_include_p doesn't think they're the same.

I tried having a magic search path entry that caused
redundant_include_p to compare pathnames, but it didn't work: under
some conditions (see the comment above r_i_p in the patch) it would
pick the wrong file to re-include.

A fix would involve finding another way to compare directory
identities.  I think that would mean having another hash table on
directory names, and even then I'm not convinced it would get it right
all the time (consider #include "../dir/file").

>> - g++.old-deja/various (test for excess errors): #include_next
>> generates a pedwarn when not in a system include directory.  System
>> include directories are only activated when the target is not marked
>> C++ aware.  linux-gnu is marked C++ aware, and egcs' limits.h uses
>> #include_next.  The fix for this is to separate the concepts of `C++
>> aware' and `system headers' - one for implicit extern "C", the other
>> for turning off pedwarns.  That would take big structural changes
>> though.  (Or we could just fix limits.h.)
>
>Why is this a regression? Sounds like it was always broken.

Other people posting test summaries don't get these errors, so it's a
regression from cccp.  I should look at how cccp does it.  This isn't
a regression from previous cpplib though.

I found another variance from cpplib last night:
c-torture/noncompile/951025-1.c.  The test harness wants to see an
error message on line 2 of the file.  cpplib reports the error on
line 1.  I think cpplib is right - the file only has one line.

>> Note that I have NOT tested with --enable-c-cpplib.  IMHO
>> --enable-c-cpplib is broken anyway, and I want to fix cpplib-as-cpp
>> 
>
>We are interested in --enable-c-cpplib, and although it is broken in
>many places, I don't think #include handling is one of them. I don't
>want to approve anything that regresses it any further.

I will run a build with --enable-c-cpplib and see what happens.  But
my point was that I think it would be a better use of everyone's time
to ignore --enable-c-cpplib for awhile.  Once cpplib can replace cccp
by default, then we can go back to working on integrating it with cc1.

zw