[PATCH] [libcxx] Distinguish between MSVC, MSVCRT, and Windows-specific code

[Reid Kleckner]

This patch has lots of unrelated stuff in it.

For example, the s/__restrict__/__restrict/ patch seems like obvious
goodness and is really easy to review on its own.

However, it's mixed in with bits like the "#if 0" change to math_win32.h
which I don't understand the need for.

I'd recommend resending a list of individual patches.  For this kind of
portability work, I think it will make it easier for people testing
different configurations to identify and report any breakage.


On Thu, Aug 15, 2013 at 10:55 AM, G M <gmisocpp at gmail.com> wrote:

> Nico, you said, quoting your whole message:
>
> "You can't just intentionally break other platforms in the process. You
> should also mention that it's for Mingw. The attached patch restores the
> previous behavior. But it doesn't link, at least for me. How are you
> building libc++ with Mingw?"
>
> Your comments about breaking "other platforms" would sound better if you
> hadn't already broke "other platforms" a week ago. Which remain broken for
> all that I know.
> They'd also sound even better if they weren't ambiguous. About an
> uncommitted patch of mine trying to unbreak a committed patch of yours? Or
> what?
> They'd be better still if they weren't of alarmist language and had even
> a screen scrape error message of what "not linking" actually means.
>
> "not linking" says nothing. Undefined symbols? Multiply defined symbols?
> Symbols related to anything I actually changed in my patch?
> What platform is your focus? Testing on what platform? All things you like
> to ask but never divulge.
>
> So Nico, keeping it simple. I can't help you. Try being more helpful, less
> terse and alarmist next time. And if you were commenting on my patch, which
> is hard to tell from the terseness, try including an actual screen scrape
> of what problem you are actually seeing next time so I and anyone see can
> see if it sounds even related to my patch.
>
> Then someone has a chance of fixing it and helping you. Otherwise you just
> give the impression my patch has a problem when it might or might not
> still and that's the end of it. Not helpful.
>
> Depending on what version of mingw and clang and gnu etc. etc. you have,
> anything might not link and it might be totally fine depending on what you
> know to be already working or broken and what it is you are actually link
> against to form an .exe if at all. libcxx / clang on widnows is kina beta
> in my opinion. If someone wants to argue otherwise, I'll defer to them. But
> in the light I see it, it's beta. So being all alarmist and terse really
> just slows a slow process that has few interested people in already, down
> to a treacle; which might explain why things have been broken for a week
> and counting. It might not too.
>
> So Nico, back to where I was before your non contribution:
>
> Dear Win32/libcxx expert people, I found another small issue with my patch
> and fixed it. This patch aims to fix whatever Nico broke before and a few
> other random things besides.
> It's not without risk given my amateur hacker status and it just
> represents my own random whims and my attempts to fix build errors in tools
> that I find.
> Take each change on it's own merit and if someone wants to query each and
> every change that's fine by me.
>
> If you dear expert / or nico want to build libcxx from svn with mingw
> here's what I use today:
>
> http://sourceforge.net/projects/mingwbuilds/files/host-windows/releases/4.8.1/64-bit/threads-win32/seh/x64-4.8.1-release-win32-seh-rev3.7z/download
> But try building before my patch and after.
> Similarly do the same with VS2013 preview as I also used that.
>
> But please don't ask me how to set mingw up, I'm not an expert and it's a
> distraction. Buf if you know and have a build bot that's configured this,
> do tell, because then me and Nico can both go look at it.
>
> Here's my patch details but the code and diff file are the last word.
>
> * Small changes to __config to add what seems to be an omitted option that
> suits VS.
> * Added include to <algorithm> for MSVCRT since it seems to now use
> builtins that otherwise seem not be present.
> * lgamma cmath that seems to be needed to compile now. I forget why but
> maybe related to howard or marshal change.
> * cstdio snprintf is a macro on VS/MINGW. It causes a compile problem
> without this change.
> * cstdlib need win32 support.h not locale.h here to compile as a possible
> result of my rejigging to support nico's change.
> * limits_win32, as a result to marshall change I think. I needed to define
> __CHAR_BIT__ for VS only not clang.
> * win32 locale.h changed about __restrict I found beneficial in testing
> with VS2013 and g++ and clang++.
> * win32 support.h
> *  also to remove strtof etc. because newer versions of C bases support so
> removing them prevents duplicate errors.
> *  changed _Exit  from a macro to an inline because it was causing compile
> errors. probably should remove support IMHO.
> * support.cpp removed excess throw stuff I added sime time ago causing
> warnings and was IMHO excessively defensive.
> * some rejigs to remove nuisance warnings.
> * some 80 column width reshaping.
>
> Thanks, mingers / Win32's. May our ranks ever decrease..
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130819/527f0bb4/attachment.html>