[PATCH] D13135: [ELF2] Add support for -L and -l command line switches

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 25 08:51:20 PDT 2015

ruiu added a comment.

This patch doesn't seem very consistent with existing code in minor points such as naming convention, error handling, use of TODO, or notation used in comments. Could you please check other code and mimic what they do?

Comment at: ELF/Driver.cpp:131
@@ -97,8 +130,3 @@
   // Create a list of input files.
-  std::vector<MemoryBufferRef> Inputs;
-  for (auto *Arg : Args.filtered(OPT_INPUT)) {
-    StringRef Path = Arg->getValue();
-    Inputs.push_back(openFile(Path));
-  }
+  std::vector<MemoryBufferRef> Inputs = openInputFiles(Args);
At least for now, we don't need that separate function. We can create any function when we need it. Please keep it in-line with other code.

Comment at: ELF/Driver.h:43-45
@@ +42,5 @@
+  // Handle -l and --library command line options
+  void onOptLibrary(std::vector<MemoryBufferRef> &Inputs, StringRef Path);
+  // Hanlde input file
+  void onOptInput(std::vector<MemoryBufferRef> &Inputs, StringRef Path);
These function names are too generic. Please name them after what they actually do.


More information about the llvm-commits mailing list