[llvm-bugs] [Bug 45812] New: win: -Wnonportable-include-path vs case of drive letter

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=45812

            Bug ID: 45812
           Summary: win: -Wnonportable-include-path vs case of drive
                    letter
           Product: clang
           Version: unspecified
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Frontend
          Assignee: unassignedclangbugs at nondot.org
          Reporter: nicolasweber at gmx.de
                CC: llvm-bugs at lists.llvm.org, neeilans at live.com,
                    richard-llvm at metafoo.co.uk

Wnonportable-include-path warns if a file is included using capitalization that
doesn't match the "true" case of the filename on disk. The motivation is that
if a file called Foo.h is included as foo.h on a
case-insensitive-but-case-preserving file system (HFS+, APFS, NTFS in the
common configurations), then things will work fine locally but it'll break the
build on a system with a case-sensitive file system. This warning will catch
this and warn about it locally.

On Windows, this is implemented by opening the included file, and then calling
GetFinalPathNameByHandle() to get the "true" case of the file name.
Preprocessor::HandleHeaderIncludeOrImport() in clang/lib/Lex/PPDirectives.cpp
then checks if that real case name matches the given case name (look for
trySimplifyPath() in that same cpp file).

That generally works great, but it kind of breaks down in the following example
(which is a somewhat common pattern in clang's tests. The reason this currently
works is because lit normalizes drive case before running tests):


c:\src\llvm-project>type test.cc
#ifndef FOO
#define FOO
#include __FILE__
#endif

c:\src\llvm-project>out\gn\bin\clang-cl.exe %cd%\test.cc /c
c:\\src\\llvm-project\\test.cc(3,10): warning: non-portable path to file
'"C:\\src\\llvm-project\\test.cc"'; specified path differs in case from file
name on disk [-Wnonportable-include-path]
#include __FILE__
         ^
<scratch space>(2,1): note: expanded from here
"c:\\src\\llvm-project\\test.cc"
^ 
1 warning generated.


Note that my pwd here is "c:\src\llvm-project" wit a lower-case c:. If you do
"cd c:\sRC\llvm-pROJECT" (and you're not in that directory already), then cmd
will set your cwd to the "actual" case for all parts of the path, except for
the drive letter -- that's taken verbatim from the path passed to cd.

At the NTFS level, drive letters don't really exist: You can pass several
options to GetFinalPathNameByHandle(). Here's some possible outputs using
https://github.com/nico/hack/blob/master/win32/getfilename.cc :


c:\src\llvm-project>getfilename --volume=dos %cd%\test.cc
\\?\C:\src\llvm-project\test.cc
c:\src\llvm-project>getfilename --volume=nt %cd%\test.cc
\Device\HarddiskVolume4\src\llvm-project\test.cc
c:\src\llvm-project>getfilename --volume=guid %cd%\test.cc
\\?\Volume{73a7f537-a778-4485-aa2a-cb6f79f27fcc}\src\llvm-project\test.cc
c:\src\llvm-project>getfilename --volume=none %cd%\test.cc
\src\llvm-project\test.cc



We use the =dos version in LLVM and strip the \\?\ prefix.

So GetFinalPathNameByHandle() always claims that disk letters are upper-case,
but the cwd in cmd.exe can have lower or upper case drive letters. So
Wnonportable-include-path can fire spuriously, based on if the drive letter in
your cwd is upper case or lower case.

We should fix this somehow. Lit seems like the wrong place to put this logic,
since it means you'll get the warning when you copy commands lit prints to run
them locally. (And you might run into this independent from our test suite too,
of course.)


Options I can think of:

1. make __FILE__ always expand to an upper-case drive letter. I think it's good
if this returns the path exactly as the user gave it to clang, so I don't like
this option.

2. make trySimplifyPath() ignore case of the drive letter. Since the motivation
for the warning is to help cross-OS code and since drive letters aren't a thing
on non-Windows, this seems like the best option to me. (and even `fsutil file
setCaseSensitiveInfo` on Windows only works on folders, so it can't affect a
drive letter.)

3. don't emit this warning at all for absolute paths. Absolute paths are almost
by definition system-dependent, and the warning didn't work correctly in this
case for quite a while (see bug 31836). I can see the argument for this too,
but having the warning not fire based on if a path is absolute or not makes the
warning a bit mysterious and harder to understand, so I weakly prefer the
former approach.


Any opinions?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20200505/0d6a892e/attachment-0001.html>