[PATCH] Libstdc++ support changes.
Corinna Vinschen
corinna-cygwin@cygwin.com
Tue Jul 7 17:19:00 GMT 2009
Hi Dave,
Thanks for doing that stuff!
On Jul 7 17:22, Dave Korn wrote:
>
> Hi all,
>
> I just got done doing a C/C++/libstdc++-v3 test run against GCC HEAD using
> the Cygwin DLL built with these patches, and everything worked. In
> particular, it passed these tests:
>
> > FAIL: g++.old-deja/g++.abi/cxa_vec.C execution test
> > FAIL: g++.old-deja/g++.brendan/new3.C execution test
>
> ... which fail on current 4.3.2-2 using shared libstdc++ DLL precisely because
> they expect to be able to interpose libstdc++'s own internal calls to the
> allocation operators. I've also been using it in daily use (and before that,
> the previous spin of this patch) for a while now and nothing unusual has been
> showing up.
> [...]
This looks pretty good to me. I have just two formal nits.
In the ChangeLogs,
> * Makefile.common (COMPILE_CXX): Add support for per-file overrides
please use just one space after the colon.
At some points you're using different comment types rather freely.
Here's an example.
> Index: winsup/cygwin/libstdcxx_wrapper.cc
> ===================================================================
> RCS file: winsup/cygwin/libstdcxx_wrapper.cc
> diff -N winsup/cygwin/libstdcxx_wrapper.cc
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ winsup/cygwin/libstdcxx_wrapper.cc 7 Jul 2009 15:21:57 -0000
> @@ -0,0 +1,91 @@
> +/* libstdcxx_wrapper.cc
> +
> + Copyright 2009 Red Hat, Inc.
> +
> +This file is part of Cygwin.
> +
> +This software is a copyrighted work licensed under the terms of the
> +Cygwin license. Please consult the file "CYGWIN_LICENSE" for
> +details. */
^^^^^
That's ok.
> +
> +
> +/* We provide these stubs to call into a user's
> + provided ONDEE replacement if there is one - otherwise
> + it must fall back to the standard libstdc++ version.
> +*/
^^^^^
The comment closing */ should be at the end of the last line of comment,
rather than starting a new line.
> +#include "winsup.h"
> +#include "cygwin-cxx.h"
> +#include "perprocess.h"
> +
> +// We are declaring asm names for the functions we define here, as we want
> +// to define the wrappers in this file. GCC links everything with wrappers
> +// around the standard C++ memory management operators; these are the wrappers,
> +// but we want the compiler to know they are the malloc operators and not have
> +// it think they're just any old function matching 'extern "C" _wrap_*'.
^^^^^
While we have a couple of // comments in Cygwin, it would be nice to at
least don't use them for multiline comments and comments on their own
line. Use
/* This is a comment. */
/* This is
another comment. */
instead.
Other than that it looks like you tested this a lot so it's fine with
me. Maybe Chris has some additional comment.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Project Co-Leader cygwin AT cygwin DOT com
Red Hat
More information about the Cygwin-patches
mailing list