Fix path concatenation in HeaderSearch

[yaron.keren]

Summary

Fixed the FIXME: Filename concatenation done using llvm::sys::path::append.

[• rafael]

Does this fix any bug or is just a cleanup? If it fixes a bug, can you
include a testcase?

[yaron.keren]

I fixes the file names shown in clang messages as

c:\my\dir/file.cpp

(incosistent slashes) TmpDir arrives in native so always concatenating '/' is wrong. That's what the fixme said...

testcase is tricky as the path seperator changes if clang and the tools were compiled on windows or not. It does not depend on the target but on the host.

Can we make a test that behave differently based on the host OS?

[• rafael]

Can we make a test that behave differently based on the host OS?

Since the behavior change is on windows, I think you can just write a test with

REQUIRES: system-windows

Cheers,
Rafael

[yaron.keren]

// REQUIRES: system-windows

run on Windows 7 results in

UNSUPPORTED: LLVM :: Other/path-seperator.c (1 of 1)

what feature work with requires? In other test I have seen REQUIRES: asserts, zlib, nozlib but no OS example.

[yaron.keren]

My mistake, I ran the test under llvm/test instead of clang/test.

[yaron.keren]

+deleted line
+test

[yaron.keren]

This patch caused one regression test failure, clang/test/Misc/remap-file.c.

The failure is the result of a FileManager::getVirtualFile limitation: it considers c:\dir\file different than c:/dir/file.
FileManager::getFile may have the same problem but I think it gets sorted out since both names point to same inode and getFile merges them.

Regardless of this patch I think getVirtualFile and getFile should be tolerant to the path seperators.

How to proceed?
Fix the get*File to be tolerant? his will require allocating a native version filename for each entry.
XFAIL the test on Windows and add a Windows only version of it?

[• rafael]

I added Argyrios Kyrtzidis to the review since he has done some work on clang's file handling. He probably has an opinion on how to fix the virtual interface.

[yaron.keren]

It may make sense to fix this together with file names case insensitivty on Windows

http://llvm.org/bugs/show_bug.cgi?id=17993

[yaron.keren]

Continued in http://reviews.llvm.org/D6871

tools/clang/lib/Lex/HeaderSearch.cpp

          I != E; ++I) {
       const FileEntry *Includer = *I;
       // Concatenate the requested file onto the directory.
-      // FIXME: Portability.  Filename concatenation should be in sys::Path.
       TmpDir = Includer->getDir()->getName();
-      TmpDir.push_back('/');
-      TmpDir.append(Filename.begin(), Filename.end());
+      llvm::sys::path::append(TmpDir, Filename);
 
       // FIXME: We don't cache the result of getFileInfo across the call to
       // getFileAndSuggestModule, because it's a reference to an element of

tools/clang/test/Frontend/Inputs/just-warning.h

+#warning 1

tools/clang/test/Frontend/path-seperator.c

+// REQUIRES: system-windows
+// RUN: %clang -c %s -nostdinc -I%S\Inputs 2>&1 | FileCheck %s
+
+// CHECK: Inputs\just-warning.h:1:2: warning
+
+#include <just-warning.h>