On Aug 15, 2013, at 1:55 PM, G M <[email protected]> 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.. > > > <libcxx4.diff>_______________________________________________
Sorry for the delay in reviewing this patch. Looks fine to me. I'm counting on all those interested in this platform to have reviewed this by now. I have one tiny nit: I'd prefer in <algorithm> to move: #ifdef _LIBCPP_MSVCRT #include "support\win32\support.h" // __builtin #endif up in the header just a few lines just under: #if defined(__IBMCPP__) #include "support/ibm/support.h" #endif and just above: #include <__undef_min_max> That is: #if defined(__IBMCPP__) #include "support/ibm/support.h" #endif #ifdef _LIBCPP_MSVCRT #include "support\win32\support.h" // __builtin #endif #include <__undef_min_max> Any problems with that? Just trying to keep the includes together and I'd like to have __undef_min_max last. Otherwise, I'm ready to commit this. Thanks for working on it! Howard _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
