[PATCH] D138709: Reland "[Lex] Fix suggested spelling of /usr/bin/../include/foo"
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 25 06:12:59 PST 2022
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This reverts commit 1dc0a1e5d220b83c1074204bd3afd54f3bac4270 <https://reviews.llvm.org/rG1dc0a1e5d220b83c1074204bd3afd54f3bac4270>.
Failures were caused by unintentional conversion to native slashes by
remove_dots, so undo that: we always suggest posix slashes for includes.
This could potentially be a change in behavior on windows if people were
spelling headers with backslashes and headermaps contained backslashes,
but that's all underspecified and I don't think anyone uses headermaps
on windows.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138709
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/unittests/Lex/HeaderSearchTest.cpp
Index: clang/unittests/Lex/HeaderSearchTest.cpp
===================================================================
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -150,6 +150,14 @@
"z");
}
+TEST_F(HeaderSearchTest, BothDotDots) {
+ addSearchDir("/x/../y/");
+ EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/x/../y/z",
+ /*WorkingDir=*/"",
+ /*MainFile=*/""),
+ "z");
+}
+
TEST_F(HeaderSearchTest, IncludeFromSameDirectory) {
EXPECT_EQ(Search.suggestPathToFileForDiagnostics("/y/z/t.h",
/*WorkingDir=*/"",
Index: clang/lib/Lex/HeaderSearch.cpp
===================================================================
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1928,32 +1928,27 @@
llvm::StringRef File, llvm::StringRef WorkingDir, llvm::StringRef MainFile,
bool *IsSystem) {
using namespace llvm::sys;
+
+ llvm::SmallString<32> FilePath = File;
+ // remove_dots switches to backslashes on windows as a side-effect!
+ // We always want to suggest forward slashes for includes.
+ path::remove_dots(FilePath, /*remove_dot_dot=*/true);
+ path::native(FilePath, path::Style::posix);
+ File = FilePath;
unsigned BestPrefixLength = 0;
// Checks whether `Dir` is a strict path prefix of `File`. If so and that's
// the longest prefix we've seen so for it, returns true and updates the
// `BestPrefixLength` accordingly.
- auto CheckDir = [&](llvm::StringRef Dir) -> bool {
- llvm::SmallString<32> DirPath(Dir.begin(), Dir.end());
+ auto CheckDir = [&](llvm::SmallString<32> Dir) -> bool {
if (!WorkingDir.empty() && !path::is_absolute(Dir))
- fs::make_absolute(WorkingDir, DirPath);
- path::remove_dots(DirPath, /*remove_dot_dot=*/true);
- Dir = DirPath;
+ fs::make_absolute(WorkingDir, Dir);
+ path::remove_dots(Dir, /*remove_dot_dot=*/true);
for (auto NI = path::begin(File), NE = path::end(File),
DI = path::begin(Dir), DE = path::end(Dir);
- /*termination condition in loop*/; ++NI, ++DI) {
- // '.' components in File are ignored.
- while (NI != NE && *NI == ".")
- ++NI;
- if (NI == NE)
- break;
-
- // '.' components in Dir are ignored.
- while (DI != DE && *DI == ".")
- ++DI;
+ NI != NE; ++NI, ++DI) {
if (DI == DE) {
- // Dir is a prefix of File, up to '.' components and choice of path
- // separators.
+ // Dir is a prefix of File, up to choice of path separators.
unsigned PrefixLength = NI - path::begin(File);
if (PrefixLength > BestPrefixLength) {
BestPrefixLength = PrefixLength;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138709.477937.patch
Type: text/x-patch
Size: 2851 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221125/6a3aa300/attachment-0001.bin>
More information about the cfe-commits
mailing list