On 10/06/2016 11:37 AM, Andris Pavenis wrote:
On 09/08/2016 12:09 PM, Thomas Schwinge wrote:
Hi!
A few review comments:
On Wed, 7 Sep 2016 20:19:20 +0300, Andris Pavenis
<andris.pave...@iki.fi> wrote:
This patch fixes handling header.gcc in subdirectories when command
line option -remap has been
used.
(I have not yet looked up what that mechanism actually does.) ;-)
Current version finds header.gcc in directories from GCC include
directory search path but
fails to find them in subdirectories due to missing directory separator.
Can you provide some test cases? (Ah, I now see you got some "Test
script to reproduce problem" attached to <https://gcc.gnu.org/PR71681> --
this should be turned into a regression test for the GCC testsuite.)
Separate patch with test-cases is in the attachment.
I added also test for header.gcc directly in directory on GCC includes
search path. This case have always worked but there is no test-case for
it in gcc test-suite yet.
Other test case (pr71681-2.c) fails without initial patch for
libcpp/files.c
(https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00395.html) applied but
passes after it is applied.
I would prefer to applied this new patch at first and initial patch
after that if they are acceptable.
Andris
ChangeLog entry for new patch:
2016-10-06 Andris Pavenis <andris.pave...@iki.fi> Add test-cases for
pr71681
* gcc.dg/cpp/pr71681-1.c: New testcase (-remap, header.gcc in directory
on includes search path)
* gcc.dg/cpp/pr71681-2.c: New testcase (-remap, header.gcc in subdirectory)
* gcc.dg/cpp/remap/header.gcc: File for added test-cases
* gcc.dg/cpp/remap/a/header.gcc: Likewise
* gcc.dg/cpp/remap/a/t_1.h: Likewise
* gcc.dg/cpp/remap/a/t_2.h: Likewise
The testsuite fixes are fine. However, please don't check them in until
you've also checked in the actual fix to remap_filename.
In your change to remap_file you have:
> + if (dir->len && !IS_DIR_SEPARATOR(dir->name[dir->len - 1]))
Please insert a space after IS_DIR_SEPARATOR, but before the open
parenthesis.
Your change to remap_filename is OK with that formatting nit fixed.
Thanks for your patience,
jeff
