- User Since
- Aug 21 2015, 4:29 PM (241 w, 4 d)
Mon, Mar 30
@vchuravy LGTM. Just to note an alternative to this patch would be to implement a "barebones" version of xcrun that implements enough functionality to report the SDK to CMake. I think the compromise we came to is fine for now. If this patch becomes a maintenance burden in the future then we may need to revisit this.
Sun, Mar 29
Fri, Mar 27
@yln Thanks for the review!
Thu, Mar 26
@ldionne Thanks for addressing my comments. The patch looks like its in very good shape. I have a minor nit about the documentation. Other than that LGTM.
Wed, Mar 25
Clear error_message_buffer unconditionally.
@vitalybuka Thanks for the initial review. I've tried to address your comments. Please let me know if there's anything else you'd like to change.
Address review feedback
Tue, Mar 24
@vitalybuka Thanks for the review. I just discovered that GetEnvP() needs to be protected rather than private so it can be called by subclass implementations so I've updated this patch to do that. If you're happy with this could you approve again?
Make GetEnvP() protected rather private so sub-class implementation can call the parent function.
Mon, Mar 23
Fri, Mar 20
Thu, Mar 19
- The feature isn't documented.
Where can I document it? I'm happy to do that.
Tue, Mar 17
I agree we should check for this, however it's not clear to me this is going to make a noticeable perf difference in practice. The tests we run involve executing processes and doing things that are a lot more expensive than the simple lit substitution, even if we check for non-terminating substitutions. I think it should be possible to make it fairly quick in the usual case where full substitution happens after exactly one substitution.
Sat, Mar 14
While I can see the appeal of this I need convincing that this actually needed. I see too many downsides
Mar 5 2020
LGTM. Thanks for addressing all the issues I raised.
Feb 26 2020
Feb 23 2020
@ychen I need convincing that this feature is actually needed. Today the problem of multiple configurations is solved at the CMake level in compiler-rt where we have CMake generate multiple config directories that point to the same source tree. Each config is generated differently depending on the use case. Why isn't that sufficient for the problem you are trying to solve.
Seems okay but I think you should check with @kledzik before landing.
Feb 19 2020
Feb 18 2020
Feb 17 2020
Feb 14 2020
Feb 11 2020
Feb 4 2020
Jan 31 2020
@steven_wu Thanks for fixing the text.
Jan 28 2020
@steven_wu Please fix the description and commit message to address my questions. I don't think the original is clear enough.
@steven_wu I'm having difficultly understanding the description.
Jan 24 2020
Jan 23 2020
Explicitly define LIBFUZZER_TEST_COMPILER
Jan 22 2020
- Delete FIXME
- space tweak
Make change NFC by not changing directory or lit test prefix.
Jan 17 2020
Jan 10 2020
@yln The description of this change needs improving. The title probably ought to be "[TSan] The "thread_terminate" event is always delivered on the terminating thread" and then that should be followed with a brief justification along with a radar number (if there is one).
Dec 20 2019
Dec 16 2019
LGTM except the possible improvement to the error message.