Bug 109932

Given that more and more C++11 features will be coming into WebKit2[1] I think it would make sense to support that by compiling WebKit2 with C++11 support enabled. At the moment this limits us to choose between two compilers - GCC >= 4.7 and Clang >= 3.0. Given the near stable release branching I'd like to see this get included, the reasoning being that it would probably ease merging fixes in WebKit2 into the stable branch. I'm not sure though how this would pass through with packagers and distributors. Could such an abrupt and late decision cause problems for them? The developers on Windows (based on the information I've gathered so far) can use GCC 4.7 with C++11 support. I'd still like to double-check with them though. [1] https://lists.webkit.org/pipermail/webkit-dev/2013-January/023527.html

Created attachment 188549 [details] Provisional patch Just a provisional patch, I'd like to first clear things up regarding the possibility of already restricting for the stable release.

Comment on attachment 188549 [details] Provisional patch View in context: https://bugs.webkit.org/attachment.cgi?id=188549&action=review I think this is a pretty good decision. I guess it deserves a mail to the list though. > Source/autotools/CheckSystemAndBasicDependencies.m4:96 > +# Check that an appropriate C compiler is available. > +AC_LANG_PUSH([C]) > +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([ > +#if !(defined(__GNUC__) && !defined(__clang__) && __GNUC__ >= 4 && __GNUC_MINOR__ >= 7) \ > + && !(defined(__clang__) && __clang_major__ >= 3 && __clang_minor__ >= 0) > +#error Unsupported compiler > +#endif > +],[])],[],[AC_MSG_ERROR([Compiler GCC >= 4.7 or Clang >= 3.0 is required for C compilation])]) > +AC_LANG_POP([C]) Hrm. There's really no macro for checking via the command-line? > Source/autotools/CheckSystemAndBasicDependencies.m4:111 > +# Use C99 as the language standard for C code. > +CFLAGS="$CFLAGS -std=c99" > +# Do not warn about C++11 incompatibilities and extensions. > +CXXFLAGS="$CXXFLAGS -Wno-c++11-compat -Wno-c++11-extensions" See below. > Source/autotools/CheckSystemAndBasicDependencies.m4:127 > +# Clang requires suppression of unused arguments warning. > +if test "$CC" = "clang"; then > + CFLAGS="$CFLAGS -Qunused-arguments" > +fi > +# libstdc++ is at the moment the only option as the C++ standard library. > +if test "$CXX" = "clang++"; then Probably good to move these to SetupCompilerFlags.m4

Comment on attachment 188549 [details] Provisional patch View in context: https://bugs.webkit.org/attachment.cgi?id=188549&action=review >> Source/autotools/CheckSystemAndBasicDependencies.m4:96 >> +AC_LANG_POP([C]) > > Hrm. There's really no macro for checking via the command-line? What kind of check that would be performed via command line did you have in mind? >> Source/autotools/CheckSystemAndBasicDependencies.m4:127 >> +if test "$CXX" = "clang++"; then > > Probably good to move these to SetupCompilerFlags.m4 Yup.

Comment on attachment 188549 [details] Provisional patch View in context: https://bugs.webkit.org/attachment.cgi?id=188549&action=review >>> Source/autotools/CheckSystemAndBasicDependencies.m4:96 >>> +AC_LANG_POP([C]) >> >> Hrm. There's really no macro for checking via the command-line? > > What kind of check that would be performed via command line did you have in mind? It seems there are some autoconf macros that depend on the -dumpversion flag, but I don't really know if that's better or worse than this. :)

Comment on attachment 188549 [details] Provisional patch View in context: https://bugs.webkit.org/attachment.cgi?id=188549&action=review >>>> Source/autotools/CheckSystemAndBasicDependencies.m4:96 >>>> +AC_LANG_POP([C]) >>> >>> Hrm. There's really no macro for checking via the command-line? >> >> What kind of check that would be performed via command line did you have in mind? > > It seems there are some autoconf macros that depend on the -dumpversion flag, but I don't really know if that's better or worse than this. :) Looking only at the output it seems this simply prints out `__GNUC__.__GNUC_MINOR__` in gcc/g++ and `__GNUC__.__GNUC_MINOR__.__GNUC_PATCHLEVEL__` in clang/clang++. The problem is that Clang compilers define those macros as well, for compatibility, but with Clang 3.0 the output is 4.2.1 which is not helpful. That's why the AC_COMPILE_IFELSE macros work on code that strictly rules out Clang in case of a defined __GNUC__ macro. Note that the Intel compilers do the same thing so I'll add a check to rule them out as well. http://nadeausoftware.com/articles/2012/10/c_c_tip_how_detect_compiler_name_and_version_using_compiler_predefined_macros > Source/autotools/CheckSystemAndBasicDependencies.m4:101 > +#if !(defined(__GNUC__) && !defined(__clang__) && __GNUC__ >= 4 && __GNUC_MINOR__ >= 7) \ I'll add a further define check for the __GNUG__ macro which should be defined when compiling C++ with GCC. Just in case, really, but I'd like to be thorough here.

Created attachment 188618 [details] Patch

I approve of this change with both my webkitgtk and debian hats on. gcc 4.7 will be in the next stable release for debian, which is the only one we should care about from now on anyway, so green light from me.

Comment on attachment 188618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=188618&action=review > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). You'll need to remove this line.

(In reply to comment #8) > I approve of this change with both my webkitgtk and debian hats on. gcc 4.7 will be in the next stable release for debian, which is the only one we should care about from now on anyway, so green light from me. Thanks! Just to clarify the stable release reference, is this OK to merge into the webkit-2.0 branch?

(In reply to comment #10) > (In reply to comment #8) > > I approve of this change with both my webkitgtk and debian hats on. gcc 4.7 will be in the next stable release for debian, which is the only one we should care about from now on anyway, so green light from me. > > Thanks! Just to clarify the stable release reference, is this OK to merge into the webkit-2.0 branch? I think we should do it.

(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #8) > > > I approve of this change with both my webkitgtk and debian hats on. gcc 4.7 will be in the next stable release for debian, which is the only one we should care about from now on anyway, so green light from me. > > > > Thanks! Just to clarify the stable release reference, is this OK to merge into the webkit-2.0 branch? > > I think we should do it. why?

(In reply to comment #12) > > I think we should do it. > > why? If we did this at 2.2 we'd be raising our dependency requirements before 3.0, so doing it for 2.0 means we don't have to wait until 3.0.

(In reply to comment #13) > (In reply to comment #12) > > > > I think we should do it. > > > > why? > > If we did this at 2.2 we'd be raising our dependency requirements before 3.0, so doing it for 2.0 means we don't have to wait until 3.0. Is there any rule that doesn't allow us to bump requirements until we release a 3.0?

I reason the merging with the idea that since more and more WebKit2 (and someday WebCore as well) code will use C++11 goodness, along with the fact the 2.0 release series will be (AFAIK) supported in the long term, this would ease merging of future patches (security fixes etc.) into the webkit-2.0 branch.

Waiting for nods from both Carlos and Gustavo on comment #10 before landing this.

(In reply to comment #16) > Waiting for nods from both Carlos and Gustavo on comment #10 before landing this. nod

(In reply to comment #16) > Waiting for nods from both Carlos and Gustavo on comment #10 before landing this. via IRC: (05:00:44 PM) kov: zdobersek, yeah, I'm OK with pushing that to webkit-2.0 \o/

Added changelogs (dunno why they weren't there in the first place) and landed in r145084. http://trac.webkit.org/changeset/145084