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>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
