thakis created this revision. thakis added a reviewer: compnerd. Herald added a subscriber: hiraditya. Herald added a project: LLVM.
Currently, normalize() for posix replaces backslashes to slashes, except that two backslashes in sequence are kept as-is. clang calls normalize() to convert \ to / is microsoft compat mode. This generally works well, but a path like "c:\\foo\\bar.h" with two backslashes doesn't work due to the exception in normalize(). These paths happen naturally on Windows hosts with e.g. `#include __FILE__`, and them not working on other hosts makes it more difficult to write tests for this case. The special case has been around without justification since this code was added in r203611 (since then moved around in r215241 r215243). No integration tests fail if I remove it. Try removing the special case. https://reviews.llvm.org/D79265 Files: clang/test/Lexer/case-insensitive-include-ms.c llvm/lib/Support/Path.cpp llvm/unittests/Support/Path.cpp Index: llvm/unittests/Support/Path.cpp =================================================================== --- llvm/unittests/Support/Path.cpp +++ llvm/unittests/Support/Path.cpp @@ -1182,7 +1182,7 @@ Tests.emplace_back("a", "a", "a"); Tests.emplace_back("a/b", "a\\b", "a/b"); Tests.emplace_back("a\\b", "a\\b", "a/b"); - Tests.emplace_back("a\\\\b", "a\\\\b", "a\\\\b"); + Tests.emplace_back("a\\\\b", "a\\\\b", "a//b"); Tests.emplace_back("\\a", "\\a", "/a"); Tests.emplace_back("a\\", "a\\", "a/"); Index: llvm/lib/Support/Path.cpp =================================================================== --- llvm/lib/Support/Path.cpp +++ llvm/lib/Support/Path.cpp @@ -539,17 +539,11 @@ PathHome.append(Path.begin() + 1, Path.end()); Path = PathHome; } - } else { - for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI) { - if (*PI == '\\') { - auto PN = PI + 1; - if (PN < PE && *PN == '\\') - ++PI; // increment once, the for loop will move over the escaped slash - else - *PI = '/'; - } - } + return; } + for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI) + if (*PI == '\\') + *PI = '/'; } std::string convert_to_slash(StringRef path, Style style) { Index: clang/test/Lexer/case-insensitive-include-ms.c =================================================================== --- clang/test/Lexer/case-insensitive-include-ms.c +++ clang/test/Lexer/case-insensitive-include-ms.c @@ -6,15 +6,17 @@ // RUN: %clang_cc1 -fsyntax-only -fms-compatibility %s -include %s -I %t/Output -verify // RUN: %clang_cc1 -fsyntax-only -fms-compatibility -fdiagnostics-parseable-fixits %s -include %s -I %t/Output 2>&1 | FileCheck %s -// FIXME: Add a test with repeated backslashes once clang can handle that -// in ms-compat mode on non-Windows hosts. #include "..\Output\.\case-insensitive-include.h" #include "..\Output\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\"" +#include "..\\Output\.\\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"..\\\\Output\\.\\\\case-insensitive-include.h\"" #include "..\output\.\case-insensitive-include.h" // expected-warning {{non-portable path}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\"" #include "apath\..\.\case-insensitive-include.h" #include "apath\..\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath\\..\\.\\case-insensitive-include.h\"" +#include "apath\\..\\.\\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"apath\\\\..\\\\.\\\\case-insensitive-include.h\"" #include "APath\..\.\case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-(
Index: llvm/unittests/Support/Path.cpp =================================================================== --- llvm/unittests/Support/Path.cpp +++ llvm/unittests/Support/Path.cpp @@ -1182,7 +1182,7 @@ Tests.emplace_back("a", "a", "a"); Tests.emplace_back("a/b", "a\\b", "a/b"); Tests.emplace_back("a\\b", "a\\b", "a/b"); - Tests.emplace_back("a\\\\b", "a\\\\b", "a\\\\b"); + Tests.emplace_back("a\\\\b", "a\\\\b", "a//b"); Tests.emplace_back("\\a", "\\a", "/a"); Tests.emplace_back("a\\", "a\\", "a/"); Index: llvm/lib/Support/Path.cpp =================================================================== --- llvm/lib/Support/Path.cpp +++ llvm/lib/Support/Path.cpp @@ -539,17 +539,11 @@ PathHome.append(Path.begin() + 1, Path.end()); Path = PathHome; } - } else { - for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI) { - if (*PI == '\\') { - auto PN = PI + 1; - if (PN < PE && *PN == '\\') - ++PI; // increment once, the for loop will move over the escaped slash - else - *PI = '/'; - } - } + return; } + for (auto PI = Path.begin(), PE = Path.end(); PI < PE; ++PI) + if (*PI == '\\') + *PI = '/'; } std::string convert_to_slash(StringRef path, Style style) { Index: clang/test/Lexer/case-insensitive-include-ms.c =================================================================== --- clang/test/Lexer/case-insensitive-include-ms.c +++ clang/test/Lexer/case-insensitive-include-ms.c @@ -6,15 +6,17 @@ // RUN: %clang_cc1 -fsyntax-only -fms-compatibility %s -include %s -I %t/Output -verify // RUN: %clang_cc1 -fsyntax-only -fms-compatibility -fdiagnostics-parseable-fixits %s -include %s -I %t/Output 2>&1 | FileCheck %s -// FIXME: Add a test with repeated backslashes once clang can handle that -// in ms-compat mode on non-Windows hosts. #include "..\Output\.\case-insensitive-include.h" #include "..\Output\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\"" +#include "..\\Output\.\\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"..\\\\Output\\.\\\\case-insensitive-include.h\"" #include "..\output\.\case-insensitive-include.h" // expected-warning {{non-portable path}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\"" #include "apath\..\.\case-insensitive-include.h" #include "apath\..\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath\\..\\.\\case-insensitive-include.h\"" +#include "apath\\..\\.\\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"apath\\\\..\\\\.\\\\case-insensitive-include.h\"" #include "APath\..\.\case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-(
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits