Author: tasiraj Date: Fri Aug 9 14:24:05 2013 New Revision: 188094 URL: http://llvm.org/viewvc/llvm-project?rev=188094&view=rev Log: cpp11-migrate: Fixed path problem with include/exclude paths
This fixes a problem when the path separator in the include/exclude directory is different (e.g. "\" vs. "/") from the path separator in the file path we are modifying. Differential Revision: http://llvm-reviews.chandlerc.com/D1326 Modified: clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.cpp clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.h clang-tools-extra/trunk/unittests/cpp11-migrate/IncludeExcludeTest.cpp Modified: clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.cpp?rev=188094&r1=188093&r2=188094&view=diff ============================================================================== --- clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.cpp (original) +++ clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.cpp Fri Aug 9 14:24:05 2013 @@ -22,17 +22,17 @@ using namespace llvm; +/// A string type to represent paths. +typedef SmallString<64> PathString; + namespace { /// \brief Helper function to determine whether a file has the same path /// prefix as \a Path. /// -/// \a File shouldn't contain relative operators i.e. ".." or "." since Path -/// comes from the include/exclude list of paths in which relative operators -/// were removed. /// \a Path must be an absolute path. bool fileHasPathPrefix(StringRef File, StringRef Path) { // Converts File to its absolute path. - SmallString<64> AbsoluteFile = File; + PathString AbsoluteFile = File; sys::fs::make_absolute(AbsoluteFile); // Convert path strings to sys::path to iterate over each of its directories. @@ -43,7 +43,10 @@ bool fileHasPathPrefix(StringRef File, S while (FileI != FileE && PathI != PathE) { // If the strings aren't equal then the two paths aren't contained within // each other. - if (!FileI->equals(*PathI)) + bool IsSeparator = ((FileI->size() == 1) && (PathI->size() == 1) && + sys::path::is_separator((*FileI)[0]) && + sys::path::is_separator((*PathI)[0])); + if (!FileI->equals(*PathI) && !IsSeparator) return false; ++FileI; ++PathI; @@ -53,6 +56,7 @@ bool fileHasPathPrefix(StringRef File, S /// \brief Helper function for removing relative operators from a given /// path i.e. "..", ".". +/// \a Path must be a absolute path. std::string removeRelativeOperators(StringRef Path) { sys::path::const_iterator PathI = sys::path::begin(Path); sys::path::const_iterator PathE = sys::path::end(Path); @@ -68,7 +72,7 @@ std::string removeRelativeOperators(Stri ++PathI; } // Rebuild the new path. - SmallString<64> NewPath; + PathString NewPath; for (SmallVectorImpl<StringRef>::iterator I = PathT.begin(), E = PathT.end(); I != E; ++I) { llvm::sys::path::append(NewPath, *I); @@ -86,7 +90,7 @@ error_code parseCLInput(StringRef Line, E = Tokens.end(); I != E; ++I) { // Convert each path to its absolute path. - SmallString<64> Path = I->rtrim(); + PathString Path = I->rtrim(); if (error_code Err = sys::fs::make_absolute(Path)) return Err; // Remove relative operators from the path. Modified: clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.h?rev=188094&r1=188093&r2=188094&view=diff ============================================================================== --- clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.h (original) +++ clang-tools-extra/trunk/cpp11-migrate/Core/IncludeExcludeInfo.h Fri Aug 9 14:24:05 2013 @@ -42,6 +42,10 @@ public: /// \brief Determine if the given path is in the list of include paths but /// not in the list of exclude paths. + /// + /// \a FilePath shouldn't contain relative operators i.e. ".." or "." since + /// Path comes from the include/exclude list of paths in which relative + /// operators were removed. bool isFileIncluded(llvm::StringRef FilePath) const; private: Modified: clang-tools-extra/trunk/unittests/cpp11-migrate/IncludeExcludeTest.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/cpp11-migrate/IncludeExcludeTest.cpp?rev=188094&r1=188093&r2=188094&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/cpp11-migrate/IncludeExcludeTest.cpp (original) +++ clang-tools-extra/trunk/unittests/cpp11-migrate/IncludeExcludeTest.cpp Fri Aug 9 14:24:05 2013 @@ -28,7 +28,7 @@ TEST(IncludeExcludeTest, ParseString) { IncludeExcludeInfo IEManager; llvm::error_code Err = IEManager.readListFromString( - /*include=*/ "a,b/b2,c/c2", + /*include=*/ "a,b/b2,c/c2,d/../d2/../d3", /*exclude=*/ "a/af.cpp,a/a2,b/b2/b2f.cpp,c/c2"); ASSERT_EQ(Err, llvm::error_code::success()); @@ -37,11 +37,14 @@ TEST(IncludeExcludeTest, ParseString) { // transform. Files are not safe to transform by default. EXPECT_FALSE(IEManager.isFileIncluded("f.cpp")); EXPECT_FALSE(IEManager.isFileIncluded("b/dir/f.cpp")); + EXPECT_FALSE(IEManager.isFileIncluded("d/f.cpp")); + EXPECT_FALSE(IEManager.isFileIncluded("d2/f.cpp")); // If the file appears on only the include list then it is safe to transform. EXPECT_TRUE(IEManager.isFileIncluded("a/f.cpp")); EXPECT_TRUE(IEManager.isFileIncluded("a/dir/f.cpp")); EXPECT_TRUE(IEManager.isFileIncluded("b/b2/f.cpp")); + EXPECT_TRUE(IEManager.isFileIncluded("d3/f.cpp")); // If the file appears on both the include or exclude list then it is not // safe to transform. @@ -50,6 +53,21 @@ TEST(IncludeExcludeTest, ParseString) { EXPECT_FALSE(IEManager.isFileIncluded("a/a2/dir/f.cpp")); EXPECT_FALSE(IEManager.isFileIncluded("b/b2/b2f.cpp")); EXPECT_FALSE(IEManager.isFileIncluded("c/c2/c3/f.cpp")); + +#ifdef LLVM_ON_WIN32 + // Check for cases when the path separators are different between the path + // that was read and the path that we are checking for. This can happen on + // windows where lit provides "\" and the test has "/". + ASSERT_NO_ERROR(IEManager.readListFromString( + /*include=*/ "C:\\foo,a\\b/c,a/../b\\c/..\\d", + /*exclude=*/ "C:\\bar" + )); + EXPECT_TRUE(IEManager.isFileIncluded("C:/foo/code.h")); + EXPECT_FALSE(IEManager.isFileIncluded("C:/bar/code.h")); + EXPECT_TRUE(IEManager.isFileIncluded("a/b\\c/code.h")); + EXPECT_FALSE(IEManager.isFileIncluded("b\\c/code.h")); + EXPECT_TRUE(IEManager.isFileIncluded("b/d\\code.h")); +#endif } TEST(IncludeExcludeTest, ParseStringCases) { _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
