- User Since
- Apr 18 2013, 12:16 AM (253 w, 1 d)
That's weird, because lots of lldb tests compile and link test binaries on Windows with -fuse-ld=lld (without the .exe). What makes you say the .exe is necessary?
Yes, there's a chance that this test succeeds but the final output fails, but that should be acceptable. File writing could fail for other reasons, like disk full situation, and there's no way to know that before we actually write bytes to a file, so I think it's not worth to spend too much effort to be more accurate.
LGTM with this fix.
Yeah, please add a test for this for demonstration purpose. Actually, for demonstration purpose, a test is probably more useful than the actual code, because we are more interested in knowing about how a program works as a result of a change than the change itself. In particular, I'm interested in knowing the information that is contained in the new output.
- updated the test case.
(So my previous comments were optional -- you can submit without fixing them.)
There are a few things that hasn't been addressed, but we can address them later if we still want to do that.
Maybe we can just say "done"? No products are shipped without bugs, so having a few known bugs isn't that bad.
Thanks. I'm writing release notes too, so I'll merge it with yours. Please just submit yours now and I'll edit it further.
Wed, Feb 21
Tue, Feb 20
Yes, I forgot to do that. Will do next time.
As long as it is consistent, I don't care much about this style, and I always use clang-format, so I like this change.
- removed unused parameter from shouldReplace
- use toString() instead of getName().
- fix typo
Please go first.
- reduces number of checkSymbolTypes function calls.
Pardon my ignorance, but what is a difference between x86 and x86-registered-target?
Mon, Feb 19
The patch to remove OutRelocation also contained this change, so this change was submitted as part of that. I do prefer shorter functions, but in this case it feels to me that writeTo does too less. That's probably a personal preference though.
- remove dead code
I didn't actually try to see the assembly output for this file, but decent compilers are smart enough to figure out that the lambda can be inlined, so I don't think this change doesn't improve performance.
Sat, Feb 17
Fri, Feb 16
Good observation, but I think we should use toString() in ELF too. toString() is there for you so that you don't need to construct a string like this, and doing this is error-prone. In this case, if file foo.o is in bar.a, toString generates something like
But I believe a better way of doing this kind of stuff is to define toString(InputChunk *). toString() function is a common interface to stringize objects to create debug messages.
- removed --full-shutdown
Actually I've just did that a few hours ago. Please sync to SVN head.
If this is needed by all tests, how about making it an environment variable (e.g. "LLD_IN_TEST=1") and detect it in lld.cpp instead of in each driver?
- removed a comment from the .h file