[PATCH] D38842: [CrossTU] Fix handling of Cross Translation Unit directory path

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316764: [CrossTU] Fix handling of Cross Translation Unit 
directory path (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D38842?vs=118773=120576#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38842

Files:
  cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
  cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp


Index: cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
===
--- cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
+++ cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
@@ -93,10 +93,7 @@
 index_error_code::multiple_definitions, IndexPath.str(), LineNo);
   StringRef FileName = LineRef.substr(Pos + 1);
   SmallString<256> FilePath = CrossTUDir;
-  if (llvm::sys::path::is_absolute(FileName))
-FilePath = FileName;
-  else
-llvm::sys::path::append(FilePath, FileName);
+  llvm::sys::path::append(FilePath, FileName);
   Result[FunctionLookupName] = FilePath.str().str();
 } else
   return llvm::make_error(
Index: cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -109,9 +109,9 @@
 
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
-  Index["a"] = "b";
-  Index["c"] = "d";
-  Index["e"] = "f";
+  Index["a"] = "/b/f1";
+  Index["c"] = "/d/f2";
+  Index["e"] = "/f/f3";
   std::string IndexText = createCrossTUIndexString(Index);
 
   int IndexFD;
@@ -134,5 +134,25 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
+TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
+  llvm::StringMap Index;
+  Index["a"] = "/b/c/d";
+  std::string IndexText = createCrossTUIndexString(Index);
+
+  int IndexFD;
+  llvm::SmallString<256> IndexFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
+  IndexFileName));
+  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
+  IndexFile.os() << IndexText;
+  IndexFile.os().flush();
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  llvm::Expected IndexOrErr =
+  parseCrossTUIndex(IndexFileName, "/ctudir");
+  EXPECT_TRUE((bool)IndexOrErr);
+  llvm::StringMap ParsedIndex = IndexOrErr.get();
+  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+}
+
 } // end namespace cross_tu
 } // end namespace clang


Index: cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
===
--- cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
+++ cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
@@ -93,10 +93,7 @@
 index_error_code::multiple_definitions, IndexPath.str(), LineNo);
   StringRef FileName = LineRef.substr(Pos + 1);
   SmallString<256> FilePath = CrossTUDir;
-  if (llvm::sys::path::is_absolute(FileName))
-FilePath = FileName;
-  else
-llvm::sys::path::append(FilePath, FileName);
+  llvm::sys::path::append(FilePath, FileName);
   Result[FunctionLookupName] = FilePath.str().str();
 } else
   return llvm::make_error(
Index: cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -109,9 +109,9 @@
 
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
-  Index["a"] = "b";
-  Index["c"] = "d";
-  Index["e"] = "f";
+  Index["a"] = "/b/f1";
+  Index["c"] = "/d/f2";
+  Index["e"] = "/f/f3";
   std::string IndexText = createCrossTUIndexString(Index);
 
   int IndexFD;
@@ -134,5 +134,25 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
+TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
+  llvm::StringMap Index;
+  Index["a"] = "/b/c/d";
+  std::string IndexText = createCrossTUIndexString(Index);
+
+  int IndexFD;
+  llvm::SmallString<256> IndexFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
+  IndexFileName));
+  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
+  IndexFile.os() << IndexText;
+  IndexFile.os().flush();
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  llvm::Expected IndexOrErr =
+  parseCrossTUIndex(IndexFileName, "/ctudir");
+  EXPECT_TRUE((bool)IndexOrErr);
+  llvm::StringMap ParsedIndex = IndexOrErr.get();
+  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+}
+
 } // end namespace cross_tu
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D38842: [CrossTU] Fix handling of Cross Translation Unit directory path

[Peter Szecsi via Phabricator via cfe-commits]

szepet added a comment.

LGTM as well, but same note applies.


https://reviews.llvm.org/D38842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38842: [CrossTU] Fix handling of Cross Translation Unit directory path

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl accepted this revision.
r.stahl added a comment.
This revision is now accepted and ready to land.

I'm gonna go ahead and approve this now, because I reported the issue. Note 
that I'm not a regular contributor, yet!




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:99
   else
 llvm::sys::path::append(FilePath, FileName);
   Result[FunctionLookupName] = FilePath.str().str();

Here you could drop the if/else completely and just append.


https://reviews.llvm.org/D38842



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38842: [CrossTU] Fix handling of Cross Translation Unit directory path

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun created this revision.

The function map generator tool always creates absolute path. The correct logic 
to determine whether a path should be handled as absolute depends on the value 
of the CrossTU directory. Added a unit test to avoid regressions.


https://reviews.llvm.org/D38842

Files:
  lib/CrossTU/CrossTranslationUnit.cpp
  unittests/CrossTU/CrossTranslationUnitTest.cpp


Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -109,9 +109,9 @@
 
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
-  Index["a"] = "b";
-  Index["c"] = "d";
-  Index["e"] = "f";
+  Index["a"] = "/b/f1";
+  Index["c"] = "/d/f2";
+  Index["e"] = "/f/f3";
   std::string IndexText = createCrossTUIndexString(Index);
 
   int IndexFD;
@@ -134,5 +134,25 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
+TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
+  llvm::StringMap Index;
+  Index["a"] = "/b/c/d";
+  std::string IndexText = createCrossTUIndexString(Index);
+
+  int IndexFD;
+  llvm::SmallString<256> IndexFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
+  IndexFileName));
+  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
+  IndexFile.os() << IndexText;
+  IndexFile.os().flush();
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  llvm::Expected IndexOrErr =
+  parseCrossTUIndex(IndexFileName, "/ctudir");
+  EXPECT_TRUE((bool)IndexOrErr);
+  llvm::StringMap ParsedIndex = IndexOrErr.get();
+  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+}
+
 } // end namespace cross_tu
 } // end namespace clang
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -93,7 +93,7 @@
 index_error_code::multiple_definitions, IndexPath.str(), LineNo);
   StringRef FileName = LineRef.substr(Pos + 1);
   SmallString<256> FilePath = CrossTUDir;
-  if (llvm::sys::path::is_absolute(FileName))
+  if (CrossTUDir.empty())
 FilePath = FileName;
   else
 llvm::sys::path::append(FilePath, FileName);


Index: unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -109,9 +109,9 @@
 
 TEST(CrossTranslationUnit, IndexFormatCanBeParsed) {
   llvm::StringMap Index;
-  Index["a"] = "b";
-  Index["c"] = "d";
-  Index["e"] = "f";
+  Index["a"] = "/b/f1";
+  Index["c"] = "/d/f2";
+  Index["e"] = "/f/f3";
   std::string IndexText = createCrossTUIndexString(Index);
 
   int IndexFD;
@@ -134,5 +134,25 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
+TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
+  llvm::StringMap Index;
+  Index["a"] = "/b/c/d";
+  std::string IndexText = createCrossTUIndexString(Index);
+
+  int IndexFD;
+  llvm::SmallString<256> IndexFileName;
+  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
+  IndexFileName));
+  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
+  IndexFile.os() << IndexText;
+  IndexFile.os().flush();
+  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
+  llvm::Expected IndexOrErr =
+  parseCrossTUIndex(IndexFileName, "/ctudir");
+  EXPECT_TRUE((bool)IndexOrErr);
+  llvm::StringMap ParsedIndex = IndexOrErr.get();
+  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+}
+
 } // end namespace cross_tu
 } // end namespace clang
Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -93,7 +93,7 @@
 index_error_code::multiple_definitions, IndexPath.str(), LineNo);
   StringRef FileName = LineRef.substr(Pos + 1);
   SmallString<256> FilePath = CrossTUDir;
-  if (llvm::sys::path::is_absolute(FileName))
+  if (CrossTUDir.empty())
 FilePath = FileName;
   else
 llvm::sys::path::append(FilePath, FileName);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits