[PATCH] Convert CoreInputGraph.

Shankar Kalpathi Easwaran shankarke at gmail.com
Sun Dec 14 21:57:15 PST 2014


================
Comment at: include/lld/Core/File.h:270-299
@@ -268,3 +269,32 @@
+
+/// An ErrorFile represents a file that doesn't exist.
+/// If you try to parse a file which doesn't exist, an instance of this
+/// class will be returned. That's parse method always returns an error.
+/// This is useful to delay erroring on non-existent files, so that we
+/// can do unit testing a driver using non-existing file paths.
+class ErrorFile : public File {
+public:
+  ErrorFile(StringRef p, std::error_code ec) : File(p, kindObject), _ec(ec) {}
+
+  std::error_code doParse() override { return _ec; }
+
+  const atom_collection<DefinedAtom> &defined() const override {
+    llvm_unreachable("internal error");
+  }
+  const atom_collection<UndefinedAtom> &undefined() const override {
+    llvm_unreachable("internal error");
+  }
+  const atom_collection<SharedLibraryAtom> &sharedLibrary() const override {
+    llvm_unreachable("internal error");
+  }
+  const atom_collection<AbsoluteAtom> &absolute() const override {
+    llvm_unreachable("internal error");
+  }
+
+private:
+  std::error_code _ec;
+};
+
 } // end namespace lld
 
 #endif
----------------
parseFile could still return an error code. If we want to just use this class for error reporting purposes, this classes could go in the unittests for InputGraph.

================
Comment at: include/lld/Driver/WrapperInputGraph.h:22-27
@@ +21,8 @@
+
+class WrapperNode : public FileNode {
+public:
+  WrapperNode(std::unique_ptr<File> file) : FileNode(file->path()) {
+    _files.push_back(std::move(file));
+  }
+};
+
----------------
We would need to move this to CoreInputGraph.

================
Comment at: lib/Driver/CoreDriver.cpp:154-159
@@ -157,1 +153,8 @@
+    case OPT_INPUT: {
+      std::vector<std::unique_ptr<File>> files
+        = parseFile(ctx, inputArg->getValue(), false);
+      for (std::unique_ptr<File> &file : files) {
+        inputGraph->addInputElement(std::unique_ptr<InputElement>(
+            new WrapperNode(std::move(file))));
+      }
       break;
----------------
couldnt the InputElement determine that a file has not been created and automatically create a File at parsing time ?

================
Comment at: lib/Driver/Driver.cpp:10-13
@@ -9,4 +9,6 @@
 
 #include "lld/Driver/Driver.h"
+#include "lld/Core/ArchiveLibraryFile.h"
+#include "lld/Core/File.h"
 #include "lld/Core/Instrumentation.h"
 #include "lld/Core/LLVM.h"
----------------
sort includes.

================
Comment at: lib/Driver/Driver.cpp:39-50
@@ +38,14 @@
+
+FileVector parseMemberFiles(FileVector &files) {
+  std::vector<std::unique_ptr<File>> members;
+  for (std::unique_ptr<File> &file : files) {
+    if (auto *archive = dyn_cast<ArchiveLibraryFile>(file.get())) {
+      if (std::error_code ec = archive->parseAllMembers(members))
+        return makeErrorFile(file->path(), ec);
+    } else {
+      members.push_back(std::move(file));
+    }
+  }
+  return members;
+}
+
----------------
The driver should not know about ArchiveLibraryFiles. This increases the circular dependency when building lld as a shared library.

================
Comment at: lib/Driver/Driver.cpp:60-61
@@ +59,4 @@
+    return makeErrorFile(path, ec);
+  if (wholeArchive)
+    return parseMemberFiles(files);
+  return files;
----------------
FileNode::parse would need to check if wholeArchive is set and parse all members, why is this being done in the Driver ?

http://reviews.llvm.org/D6653

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list