Tooling/ToolingTests/FlushRewrittenFilesTest.GetFilePath is failing for me after this patch.
On 9 October 2013 12:09, Ariel J. Bernal <[email protected]> wrote: > Author: ajbernal > Date: Wed Oct 9 11:09:23 2013 > New Revision: 192299 > > URL: http://llvm.org/viewvc/llvm-project?rev=192299&view=rev > Log: > This patch fixes replacements that are not applied when relative paths are > specified. > > In particular it makes sure that relative paths for non-virtual files aren't > made absolute. > Added unittest test. > > Modified: > cfe/trunk/lib/Tooling/Refactoring.cpp > cfe/trunk/unittests/Tooling/RefactoringTest.cpp > > Modified: cfe/trunk/lib/Tooling/Refactoring.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring.cpp?rev=192299&r1=192298&r2=192299&view=diff > ============================================================================== > --- cfe/trunk/lib/Tooling/Refactoring.cpp (original) > +++ cfe/trunk/lib/Tooling/Refactoring.cpp Wed Oct 9 11:09:23 2013 > @@ -105,16 +105,21 @@ void Replacement::setFromSourceLocation( > const std::pair<FileID, unsigned> DecomposedLocation = > Sources.getDecomposedLoc(Start); > const FileEntry *Entry = > Sources.getFileEntryForID(DecomposedLocation.first); > - > if (Entry != NULL) { > // Make FilePath absolute so replacements can be applied correctly when > - // relative paths for files are used. > - llvm::SmallString<256> FilePath(Entry->getName()); > - llvm::error_code EC = llvm::sys::fs::make_absolute(FilePath); > - // Don't change the FilePath if the file is a virtual file. > - this->FilePath = EC ? FilePath.c_str() : Entry->getName(); > - } else > + // relative paths for files are used. But we don't want to change virtual > + // files. > + if (llvm::sys::fs::exists(Entry->getName())) { > + llvm::SmallString<256> FilePath(Entry->getName()); > + llvm::sys::fs::make_absolute(FilePath); > + this->FilePath = FilePath.c_str(); > + } > + else { > + this->FilePath = Entry->getName(); > + } > + } else { > this->FilePath = InvalidLocation; > + } > this->ReplacementRange = Range(DecomposedLocation.second, Length); > this->ReplacementText = ReplacementText; > } > > Modified: cfe/trunk/unittests/Tooling/RefactoringTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringTest.cpp?rev=192299&r1=192298&r2=192299&view=diff > ============================================================================== > --- cfe/trunk/unittests/Tooling/RefactoringTest.cpp (original) > +++ cfe/trunk/unittests/Tooling/RefactoringTest.cpp Wed Oct 9 11:09:23 2013 > @@ -244,6 +244,10 @@ public: > return Context.Sources.createFileID(File, SourceLocation(), > SrcMgr::C_User); > } > > + StringRef getFilePath(llvm::StringRef Name) { > + return TemporaryFiles.lookup(Name); > + } > + > std::string getFileContentFromDisk(llvm::StringRef Name) { > std::string Path = TemporaryFiles.lookup(Name); > assert(!Path.empty()); > @@ -270,6 +274,55 @@ TEST_F(FlushRewrittenFilesTest, StoresCh > getFileContentFromDisk("input.cpp")); > } > > +TEST_F(FlushRewrittenFilesTest, GetFilePath) { > + // Create a temporary file. > + createFile("input.cpp", "line1\nline2\nline3\nline4"); > + > + StringRef FilePath = getFilePath("input.cpp"); > + StringRef TempPath = llvm::sys::path::parent_path(FilePath); > + StringRef TempFile = llvm::sys::path::filename(FilePath); > + > + // Save current path. > + SmallString<512> CurrentPath; > + llvm::sys::fs::current_path(CurrentPath); > + > + // Change directory to the temporary directory. > + EXPECT_FALSE(chdir(TempPath.str().c_str())); > + llvm::sys::fs::current_path(CurrentPath); > + > + // Get a FileEntry from the current directory. > + FileManager Files((FileSystemOptions())); > + const FileEntry *Entry = Files.getFile(TempFile); > + ASSERT_TRUE(Entry != NULL); > + > + // We expect to get just the filename and "." in the FileEntry > + // since paths are relative to the current directory. > + EXPECT_EQ(TempFile, Entry->getName()); > + EXPECT_STREQ(".", Entry->getDir()->getName()); > + > + FileID ID = Context.Sources.createFileID(Entry, SourceLocation(), > + SrcMgr::C_User); > + > + Replacement R = Replacement(Context.Sources, Context.getLocation(ID, 2, 1), > + 5, "replaced"); > + > + // Change back to the original path so we can verify that replacements > + // are being applied independently of the location. > + EXPECT_EQ(0, chdir(CurrentPath.c_str())); > + > + // We expect that the file path of the replacement is using the absolute > + // path. > + EXPECT_EQ(R.getFilePath(), getFilePath("input.cpp")); > + > + // Apply replacements. > + Replacements Replaces; > + Replaces.insert(R); > + EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite)); > + EXPECT_FALSE(Context.Rewrite.overwriteChangedFiles()); > + EXPECT_EQ("line1\nreplaced\nline3\nline4", > + getFileContentFromDisk("input.cpp")); > +} > + > namespace { > template <typename T> > class TestVisitor : public clang::RecursiveASTVisitor<T> { > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
