[PATCH] A fix for platform-dependent types in sanitizers' profiling support lib on x64 FreeBSD in 32-bit mode
nlewycky at google.com
Mon Mar 10 14:07:23 PDT 2014
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...
More information about the llvm-commits