[PATCH] [lld] Atoms created from command line options should be available in YAML

[Rui Ueyama]

================
Comment at: include/lld/Core/LinkingContext.h:193
@@ -193,3 +192,3 @@
+  virtual void setEntrySymbolName(StringRef name) {
     // Entry function have to be resolved as an undefined symbol.
     _entrySymbolName = name;
----------------
Remove this comment as this was for the next line you just removed.

================
Comment at: include/lld/Core/LinkingContext.h:250
@@ +249,3 @@
+  /// the actual input files.
+  std::vector<std::unique_ptr<lld::File>> createInternalFiles();
+
----------------
You may want to make it virtual?

================
Comment at: include/lld/Driver/InputGraph.h:77
@@ -74,3 +76,3 @@
 
-  /// \brief Iterators
-  InputElementIterT begin() { return _inputArgs.begin(); }
+  virtual void addInternalFile(std::vector<std::unique_ptr<File> > inputFiles) {
+    for (auto &ai : inputFiles)
----------------
">>" is preferred than "> >" in LLD's coding standard.

================
Comment at: include/lld/Driver/InputGraph.h:79
@@ +78,3 @@
+    for (auto &ai : inputFiles)
+      _internalFiles.push_back(std::move(ai));
+  }
----------------
Probably doing this is better to add a vector to another vector.

 _internalFiles.insert(_internalFiles.end(), inputFiles.begin(), inputFiles.end())

================
Comment at: include/lld/ReaderWriter/ELFLinkingContext.h:176
@@ +175,3 @@
+  /// Add internal files for ELF
+  virtual std::vector<std::unique_ptr<File> > createInternalFiles();
+
----------------
>>

================
Comment at: lib/Core/LinkingContext.cpp:58
@@ +57,3 @@
+  std::unique_ptr<SimpleFile> simpleFile(
+      new SimpleFile(*this, "command line option -u"));
+  for (auto ai : _initialUndefinedSymbols)
----------------
It's not always -u. Link.exe's option is /c.

================
Comment at: lib/Core/LinkingContext.cpp:56
@@ +55,3 @@
+std::vector<std::unique_ptr<File>> LinkingContext::createInternalFiles() {
+  std::vector<std::unique_ptr<File> > result;
+  std::unique_ptr<SimpleFile> simpleFile(
----------------
>>

================
Comment at: lib/Core/LinkingContext.cpp:59
@@ +58,3 @@
+      new SimpleFile(*this, "command line option -u"));
+  for (auto ai : _initialUndefinedSymbols)
+    simpleFile->addAtom(
----------------
Is ai atom, symbol or string? I'd write the type explicitly rather than using auto because the type declaration of this data is too far from this. It's even in a different file.

================
Comment at: lib/Core/LinkingContext.cpp:66
@@ +65,3 @@
+    std::unique_ptr<SimpleFile> entryFile(
+        new SimpleFile(*this, "command line option -entry"));
+    entryFile->addAtom(
----------------
It's /entry in link.exe.

================
Comment at: lib/Core/LinkingContext.cpp:64
@@ +63,3 @@
+  StringRef entrySymbol = entrySymbolName();
+  if (!entrySymbol.empty()) {
+    std::unique_ptr<SimpleFile> entryFile(
----------------
We prefer early return (I remember this was noted somewhere in the LLVM coding standard), so it should be if (entrySymbol.empty()) return result;.

================
Comment at: lib/Driver/LDOptions.td:105
@@ -104,2 +104,3 @@
 
+
 // extensions
----------------
Is this intended?

================
Comment at: lib/ReaderWriter/ELF/File.h:754
@@ +753,3 @@
+  virtual Atom *addAbsoluteAtom(StringRef symbolName) {
+    Elf_Sym *symbol = new (_allocator.Allocate<Elf_Sym>()) Elf_Sym;
+    symbol->st_name = 0;
----------------
new (_allocator) suffices?

================
Comment at: lib/ReaderWriter/ELF/File.h:762
@@ +761,3 @@
+    auto *newAtom =
+        new (_allocator) ELFAbsoluteAtom<ELFT>(*this, symbolName, symbol, -1);
+    _absoluteAtoms._atoms.push_back(newAtom);
----------------
What does -1 mean?


http://llvm-reviews.chandlerc.com/D1566