[PATCH] A fix for platform-dependent types in sanitizers' profiling support lib on x64 FreeBSD in 32-bit mode

[Nick Lewycky]

On 10 March 2014 13:59, Justin Bogner <mail at justinbogner.com> wrote:

> Viktor Kutuzov <vkutuzov at accesssoftek.com> writes:
> >   Updated so that it does not change the set of inclusions on Windows,
> >   does not duplicate headers and pays more attention to scalability.
> >
> >   Inspiried by comments in llvm-commits by Justin Bogner:
> >
> >   > Are sys/stat.h and stdint.h not used for some reason in the
> >   > _MSC_VER case? This looks like a behaviour change that probably
> >   > breaks windows.
> >   > Duplicating the include of inttypes in _MSC_VER and in the else
> >   > seems fairly awkward here.
> >   > Overall, this approach really doesn't seem scalable at all, and it
> >   > will be very difficult for people adding things in this area to
> >   > get it right without submitting to buildbots.
> >
> >   Thanks, Justin.
> >
> >   If this version looks OK, I would appreciate if one the committers
> >   land it as I don't have the commit access. Thanks.
>
> I'd missed this update, so I apologize for the wait, but I notice that
> you've committed this in the meantime without waiting for an OK.
>
> Please don't do that in the future.
>
> It's important not to commit code until all reviewers that have raised
> concerns have acknowledged that their concerns have been addressed.
>

FYI, I had intended my email on Feb 24th to be an LGTM (it shows up as
"approved" in the code review tool though the email doesn't say that). I
haven't been following the progression of the thread after that point.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140310/d636e965/attachment.html>