- User Since
- Apr 18 2013, 12:16 AM (242 w, 4 d)
It also means that it is harder to add features in the future for things which may not yet exist. As an example, there is no equivalent to this option today, but it would be pretty nice to have: -reexport-l<ibrary>. This would be equivalent to forwarders in COFF and LC_REEXPORT in the MachO. There is no ELF equivalent, but, were we to add that to the linker, this would now require changing the entire toolchain rather than forwarding the one single option.
I wonder if replacing gnustat with shell and use [ -x <filename> ] is more robust than using gnustat. At least there's a precedence in clang/test/Driver/target-override.c.
I believe it's still fragile against weird umask setting (such as 444), but that's in practice not an issue.
--icf-data is not technically a correct name but that's probably the best name.
LGTM with this fix.
Got it. We can figure this out later.
What does this symbol do in MSVC 2017?
Got it. If you are using this option from the beginning, it should be fine.
Allowing a limited set of flags or features, such as adding a library path, is probably fine, but I'd strongly oppose to allow embedding arbitrary linker flags to object files. That would open up a can of warms as I described in the previous message.
LGTM with this change.
We used to merge data for COFF but not anymore because it causes nasty problems that are hard to find and debug. E.g. https://bugs.chromium.org/p/chromium/issues/detail?id=682773#c24
Fri, Dec 8
So is true for .so, isn't it?
Do you mind if I ask you explain the motivation of adding this feature?
I agree that log should go to stderr, but I think message should still go to stdout. We have that function to print out a regular message, such as printing out version information or some hint message. We have this function mainly because we want to acquire the global lock before printing out something in the multithreaded environment.
LGTM, but please submit this as two separate patches -- one for weak symbols and the other to fix -mtriple.
Thu, Dec 7
Thank you for doing this!
Thank you for making the change. As to the empty string handling, I wanted to make it consistent with other ports, and we in general don't write much code for edge cases to keep the code straightforward.
Wed, Dec 6
If I'm not too slow, give me a few hours to review patches. If I didn't respond timely, please go ahead without my LG. Does this sound good?
You don't need to wait for my LGTM for small-ish changes. I want to continue reviewing wasm lld patches to (1) keep wasm in line with other ports, (2) keep wasm lld as fast as possible, and (3) prevent wasm from making mistakes that other binary formats made in their history. But that's mostly about high-level design.
I believe it is me who wrote code to lowercase pathnames, but I don't remember why I decided to do that.
I just skimmed through the code, but it looks like what this code does is different from ELF. Is there a reason to be different?
- Use C macro only for the arity 2 check function.
- Use llvm::function_ref instead of std::function
How much does this buy us?
- fix typo
Tue, Dec 5
Wow. I was surprised that we didn't notice this until now. Thank you for finding this out.
LGTM but please split this into two patches, one is to change format, and the other is to remove some sections, before committing.
Let's land this so that we can experiment. I still doubt if a noncryptographic tree hash is faster than SHA1, but that can be experimented once this patch is in.