<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 3/28/2014 11:57 AM, Rui Ueyama
wrote:<br>
</div>
<blockquote
cite="mid:CAJENXgu3BotF2dq2PheXAdcQXnf_hByoHC8WY2PeXZqs0S98cA@mail.gmail.com"
type="cite">
<pre wrap="">On Fri, Mar 28, 2014 at 9:53 AM, Rui Ueyama <a class="moz-txt-link-rfc2396E" href="mailto:ruiu@google.com"><ruiu@google.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On Fri, Mar 28, 2014 at 9:42 AM, Shankar Kalpathi Easwaran <
<a class="moz-txt-link-abbreviated" href="mailto:shankarke@gmail.com">shankarke@gmail.com</a>> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">
Ok, I agree about that its more complex to have the complete --defsym
option.
Once the above comment is fixed, this patch LGTM.
================
Comment at: lib/Core/LinkingContext.cpp:59-61
@@ -58,5 +58,5 @@
-std::unique_ptr<File> LinkingContext::createUndefinedSymbolFile() const {
- return createUndefinedSymbolFile("command line option -u");
+std::unique_ptr<File> LinkingContext::createCommandLineFile() const {
+ return createCommandLineFile("<command line option -u or --defsym>");
}
----------------
Can we use a separate string for -u and --defsym. We could have a
createCommandLineFile with a StringRef like how PECOFF does, so that we can
support more command line defined by various options.
</pre>
</blockquote>
<pre wrap="">
It's actually pretty odd that we have a piece of code in LinkingContext
that creates what seems ELF-specific thing ("<command line option -u or
--defsym>"). Need to be refactored. Anyways I'll create a separate file for
--defsym.
</pre>
</blockquote>
<pre wrap="">
It turned out that it's not that easy -- each *Flavor*LinkingContext is
expected to create at most one file for its internal use. If it doesn't
need one, createCommandLIneFile return a nullptr, or a file instance
otherwise. That's working well, so we should keep it. Let's go with a
single file here.
</pre>
</blockquote>
<br>
createInternalFiles would create a separate --defsym file and add it
to the list of internal files, and append to the list of internal
files.<br>
<br>
-<br>
<br>
Shankar Easwaran<br>
<br>
<br>
<pre wrap="">
</pre>
<blockquote
cite="mid:CAJENXgu3BotF2dq2PheXAdcQXnf_hByoHC8WY2PeXZqs0S98cA@mail.gmail.com"
type="cite">
<pre wrap="">
</pre>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation</pre>
</body>
</html>