[cfe-commits] r124754 - /cfe/trunk/lib/Basic/FileManager.cpp

[Chandler Carruth]

On Wed, Feb 2, 2011 at 2:30 PM, Douglas Gregor <dgregor at apple.com> wrote:

> Author: dgregor
> Date: Wed Feb  2 16:30:17 2011
> New Revision: 124754
>
> URL: http://llvm.org/viewvc/llvm-project?rev=124754&view=rev
> Log:
> Canonicalize path names in the file manager before performing a lookup
> on that name. Canonicalization eliminates silliness such as "." and
> "foo/.." that breaks the uniquing of files in the presence of virtual
> files or files whose inode numbers have changed during
> parsing/re-parsing. c-index-test isn't able to create this crazy
> situation, so I've resorted to testing outside of the Clang
> tree. Fixes <rdar://problem/8928220>.
>
> Note that this hackery will go away once we have a real virtual file
> system on which we can layer FileManager; the virtual-files hack is
> showing cracks.
>
>
>
> Modified:
>    cfe/trunk/lib/Basic/FileManager.cpp
>
> Modified: cfe/trunk/lib/Basic/FileManager.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=124754&r1=124753&r2=124754&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
> +++ cfe/trunk/lib/Basic/FileManager.cpp Wed Feb  2 16:30:17 2011
> @@ -271,10 +271,84 @@
>   return &UDE;
>  }
>
> +/// \brief Canonicalize a file or path name by eliminating redundant
> +/// "foo/.." and "./" path components.
> +///
> +/// Uses the given scratch space to store the resulting string, if needed.
> +static llvm::StringRef CanonicalizeFileName(llvm::StringRef Filename,
> +                                            llvm::SmallVectorImpl<char>
> &Scratch) {
>

Any reason not to place this logic in LLVM's PathV2 library? Also, this is a
great candidate for a unit test. =] I suspect that even when we have proper
VFS layering, we'll still want this core functionality, just not exposed at
this layer.


> +  size_t Start = 0;
> +  bool Changed = false;
> +  do {
> +    size_t FirstSlash = Filename.find('/', Start);
> +    if (FirstSlash == llvm::StringRef::npos) {
> +      // No more components. Just copy the rest of the file name, if
> +      // we need to.
> +      if (Changed)
> +        Scratch.append(Filename.begin() + Start, Filename.end());
> +      break;
> +    }
> +
> +    if (Start + 1 == FirstSlash && Filename[Start] == '.') {
> +      // We have './'; remove it.
> +
> +      // If we haven't changed anything previously, copy the
> +      // starting bits here.
> +      if (!Changed) {
> +        Scratch.clear();
> +        Scratch.append(Filename.begin(), Filename.begin() + Start);
> +        Changed = true;
> +      }
> +
> +      // Skip over the './'.
> +      Start = FirstSlash + 1;
> +      continue;
> +    }
> +
> +    size_t SecondSlash = Filename.find('/', FirstSlash + 1);
> +    if (SecondSlash != llvm::StringRef::npos &&
> +        SecondSlash - FirstSlash == 3 &&
> +        Filename[FirstSlash + 1] == '.' &&
> +        Filename[FirstSlash + 2] == '.') {
> +      // We have 'foo/../'; remove it.
> +
> +      // If we haven't changed anything previously, copy the
> +      // starting bits here.
> +      if (!Changed) {
> +        Scratch.clear();
> +        Scratch.append(Filename.begin(), Filename.begin() + Start);
> +        Changed = true;
> +      }
> +
> +      // Skip over the 'foo/..'.
> +      Start = SecondSlash + 1;
> +      continue;
> +    }
> +
> +    if (Changed)
> +      Scratch.append(Filename.begin() + Start,
> +                     Filename.begin() + FirstSlash + 1);
> +    Start = FirstSlash + 1;
> +  } while (true);
> +
> +  if (Changed) {
> +#if 0
> +    llvm::errs() << "Canonicalized \"" << Filename << "\" to \""
> +                 << llvm::StringRef(Scratch.data(), Scratch.size()) <<
> "\"\n";
> +#endif
> +    return llvm::StringRef(Scratch.data(), Scratch.size());
> +  }
> +
> +  return Filename;
> +}
> +
>  /// getFile - Lookup, cache, and verify the specified file.  This returns
> null
>  /// if the file doesn't exist.
>  ///
>  const FileEntry *FileManager::getFile(llvm::StringRef Filename) {
> +  llvm::SmallString<128> FilenameScratch;
> +  Filename = CanonicalizeFileName(Filename, FilenameScratch);
> +
>   ++NumFileLookups;
>
>   // See if there is already an entry in the map.
> @@ -343,6 +417,9 @@
>  const FileEntry *
>  FileManager::getVirtualFile(llvm::StringRef Filename, off_t Size,
>                             time_t ModificationTime) {
> +  llvm::SmallString<128> FilenameScratch;
> +  Filename = CanonicalizeFileName(Filename, FilenameScratch);
> +
>   ++NumFileLookups;
>
>   // See if there is already an entry in the map.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110202/d139ae3a/attachment.html>