Patchwork [libcpp] Do not decrease highest_location if the included file has be included twice.

login
register
mail settings
Submitter Dodji Seketeli
Date June 4, 2013, 12:29 p.m.
Message ID <87k3magiud.fsf@seketeli.org>
Download mbox | patch
Permalink /patch/248567/
State New
Headers show

Comments

Dodji Seketeli - June 4, 2013, 12:29 p.m.
Dehao Chen <dehao@google.com> a écrit:

>> So, I'd say that in this hunk of your patch:
>>
>>> @@ -1002,7 +1002,8 @@ _cpp_stack_include (cpp_reader *pfile, const char
>>>       linemap_add is not called) or we were included from the
>>>       command-line.  */
>>>    if (file->pchname == NULL && file->err_no == 0
>>> -      && type != IT_CMDLINE && type != IT_DEFAULT)
>>> +      && type != IT_CMDLINE && type != IT_DEFAULT
>>> +      && !(file->cmacro && file->cmacro->type == NT_MACRO))
>>
>> Maybe you should test:
>>
>>     && should_stack_file (pfile, file, type == IT_IMPORT)
>>
>> rather than testing the last conjunction you added?  This is because
>> there are many conditions that could make the header to not be loaded,
>> besides the one you are testing.  Would this work in your environment?
>
> I tried to apply this change, but it failed several PCH related
> regression tests.

Of course ...  sigh.

So, should_stack_file apparently sets some state to (among other things)
flag files loaded via #import as 'imported'.  And using it here prevents
the file to be really imported later.  That kind of things.  And some
the pch related failures involve using #import.  So my idea was a bad
one.

So how about this patch then?



It passes bootstrap on x86_64-unknown-linux-gnu for all,ada,obj{c,c++}
here but then I don't know if it fixes the issue on you giant input
file.

If it does and if the maintainers think it can go in, I'll let you
handle the ChangeLog and commit.

Cheers.

Patch

diff --git a/libcpp/files.c b/libcpp/files.c
index 5c5a0b9..be80be3 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -983,6 +983,7 @@  _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets,
 {
   struct cpp_dir *dir;
   _cpp_file *file;
+  bool stacked;
 
   dir = search_path_head (pfile, fname, angle_brackets, type);
   if (!dir)
@@ -993,19 +994,26 @@  _cpp_stack_include (cpp_reader *pfile, const char *fname, int angle_brackets,
   if (type == IT_DEFAULT && file == NULL)
     return false;
 
-  /* Compensate for the increment in linemap_add that occurs in
-     _cpp_stack_file.  In the case of a normal #include, we're
-     currently at the start of the line *following* the #include.  A
-     separate source_location for this location makes no sense (until
-     we do the LC_LEAVE), and complicates LAST_SOURCE_LINE_LOCATION.
-     This does not apply if we found a PCH file (in which case
-     linemap_add is not called) or we were included from the
-     command-line.  */
+  /* Compensate for the increment in linemap_add that occurs if
+     _cpp_stack_file actually stacks the file.  In the case of a
+     normal #include, we're currently at the start of the line
+     *following* the #include.  A separate source_location for this
+     location makes no sense (until we do the LC_LEAVE), and
+     complicates LAST_SOURCE_LINE_LOCATION.  This does not apply if we
+     found a PCH file (in which case linemap_add is not called) or we
+     were included from the command-line.  */
   if (file->pchname == NULL && file->err_no == 0
       && type != IT_CMDLINE && type != IT_DEFAULT)
     pfile->line_table->highest_location--;
 
-  return _cpp_stack_file (pfile, file, type == IT_IMPORT);
+  stacked = _cpp_stack_file (pfile, file, type == IT_IMPORT);
+
+  if (!stacked)
+    /* _cpp_stack_file didn't stack the file, so let's rollback the
+       compensation dance we performed above.  */
+    pfile->line_table->highest_location++;
+
+  return stacked;
 }
 
 /* Could not open FILE.  The complication is dependency output.  */