MrTrillian added a comment.
Responded to comment.
================
Comment at: clang/test/Lexer/case-insensitive-include-win.c:5-9
+// Note: We must use the real path here, because the logic to detect case
+// mismatch relies on resolving the real path and checking that casing differs.
+// If we use %t and we are on a substitute drive S: mapping to C:\subst,
+// then we will compare "S:\test.dir\FOO.h" to "C:\subst\test.dir\foo.h"
+// and avoid emitting the diagnostic because the structure is different.
----------------
tahonermann wrote:
> MrTrillian wrote:
> > tahonermann wrote:
> > > Hmm, is it really desirable or necessary that the case mismatch logic
> > > resolve substituted drives? I wouldn't think so. This seems like another
> > > case where our common canonicalization would be desired.
> > I think it is necessary as I don't see how else we can retrieve the
> > original casing of the file path. Merely making the path absolute would not
> > do that. Searching for solutions on the internet finds ideas that are worse
> > like converting to a short path then back to a long path or rebuilding the
> > path one component at a time with a series directory listing requests.
> The warning this test checks for is diagnosed in
> `Preprocessor::HandleHeaderIncludeOrImport()`; search for
> `pp_nonportable_path` and/or `pp_nonportable_system_path`. I believe this
> warning will be issued if any component of the path does not match the case
> of the include directive. Since the file name differs in case, unless there
> is a bug in handling of the file name, this test isn't sensitive to case
> mismatches in `%t`.
>
> From a user point of view, resolving a substitute drive doesn't seem
> desirableto me with respect to determining whether a non-portable path is
> being used; I don't think the behavior should be dependent on whether a
> substitute drive is being used or what its target is.
@tahonermann I think the code is working by design and it would be an unrelated
change to modify its logic. See `trySimplifyPath` in `PPDirectives.cpp`:
```
// Below is a best-effort to handle ".." in paths. It is admittedly
// not 100% correct in the presence of symlinks.
// If these path components differ by more than just case, then we
// may be looking at symlinked paths. Bail on this diagnostic to avoid
// noisy false positives.
```
The test was previously implicitly requiring getting the realpath, and it
worked because `lit.py`'s logic of doing that. Now that requirement is explicit
in the test.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154130/new/
https://reviews.llvm.org/D154130
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
