[PATCH] Teach CC1 to dump module dependencies to a directory

[Ben Langmuir]

Hi Justin,

Thanks for working on this! It’s looking pretty close.

> +/// Append the absolute path in Nested to the path given by Root. This will
> +/// remove directory traversal from the resulting nested path.
> +static void appendNestedPath(SmallVectorImpl<char> &Root,
> +                             SmallVectorImpl<char> &Nested) {
> +  using namespace llvm::sys;
> +  SmallVector<StringRef, 16> ComponentStack;
> +
> +  StringRef Rel = path::relative_path(StringRef(Nested.begin(), Nested.size()));

You seem to have manually inlined Nested.str() ;-)  Maybe just make your Nested parameter a StringRef?

> +  // We need an absolute path to append to the root.
> +  SmallString<256> AbsoluteSrc = Src;
> +  fs::make_absolute(AbsoluteSrc);
> +  // Build the destination path.
> +  SmallString<256> Dest = Collector.getDest();
> +  size_t RootLen = Dest.size();
> +  appendNestedPath(Dest, AbsoluteSrc);

Do we need to escape this somehow on Windows, since you might get C: in the middle of your path?

And in general, will this work if Dest ends with a path separator?  Then you would end up with // in the middle, which could potentially be eaten at some point (not sure).

> +bool ModuleDependencyListener::visitInputFile(StringRef Filename, bool IsSystem,
> +                                              bool IsOverridden) {
> +  if (!Collector.insertSeen(Filename))
> +    return true;
> +  if (copyToRoot(Filename))
> +    Collector.setHasErrors();
> +  return true;
> +}

This is clearer to me if you invert the first if, but you decide.
if (Collector.insertSeen(Filename))
  if (copyToRoot(Filename))
    Collector.setHasErrors();
return true;

> +// RUN: find %t/vfs -type f | FileCheck %s -check-prefix=DUMP
> +// DUMP: AlsoDependsOnModule.framework/Headers/AlsoDependsOnModule.h
> +// DUMP: Module.framework/Frameworks/SubFramework.framework/Headers/SubFramework.h

REQUIRES: shell, since you need ‘find’.  This applies to both tests.  Also you won’t get the path separators you expect on Windows.


This isn’t really the place to discuss llvm patches, but...

> +  char *Buf = new char[BufSize];
If you don’t want to put 4 KB on the stack, how about std::vector with its data() method?

> +  for (;;) {
> +    Bytes = read(ReadFD, Buf, BufSize);
> +    if (Bytes <= 0)
> +      break;
> +    Bytes = write(WriteFD, Buf, Bytes);
> +    if (Bytes <= 0)
> +      break;
> +  }

This doesn’t seem sufficiently paranoid about the number of bytes actually written by ‘write’.

> 
> <clang-0001-Frontend-Add-a-CC1-flag-to-dump-module-dependencies.3.patch><llvm-0001-Support-Add-llvm-sys-fs-copy_file.2.patch>