Summary: | Fix gcc build after r121302 | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||
Component: | New Bugs | Assignee: | Ryosuke Niwa <rniwa> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Blocker | CC: | darin, eric, lforschler, mrowe, simon.fraser | ||||||||
Priority: | P1 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2012-06-27 00:56:14 PDT
Created attachment 149700 [details]
Patch
I don't know what they were thinking to assume RTTI is enabled for very old versions of gcc. They clearly didn't test this code with old gcc :( Comment on attachment 149700 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=149700&action=review > Source/ThirdParty/gtest/include/gtest/internal/gtest-port.h:349 > +#elif defined(__clang__) || defined(__llvm__) // Assume RTTO is always enabled on LLVM > + > +#define GTEST_HAS_RTTI 1 > + It's not correct to assume that RTTI is enabled. Doing this will break the clang build in the same way that GCC is currently broken. Comment on attachment 149700 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=149700&action=review >> Source/ThirdParty/gtest/include/gtest/internal/gtest-port.h:349 >> + > > It's not correct to assume that RTTI is enabled. Doing this will break the clang build in the same way that GCC is currently broken. Does clang also defines __GXX_RTTI flag? Clang defines it when the -fno-rtti flag is not passed. Note that we build TestWebKitAPI with -fno-rtti for both clang and gcc. I'm not quite sure which code path in gtest-port.h clang currently hits though. (In reply to comment #5) > Clang defines it when the -fno-rtti flag is not passed. Note that we build TestWebKitAPI with -fno-rtti for both clang and gcc. I'm not quite sure which code path in gtest-port.h clang currently hits though. I see some clang support in http://googletest.googlecode.com/svn/trunk/include/gtest/internal/gtest-port.h so I'll copy it over and tweak it to support old gcc. The cleanest solution is probably to just set GTEST_HAS_RTTI=0 in GCC_PREPROCESSOR_DEFINITIONS in General.xcconfig. That'll tell gtest not to use RTTI with any of our compilers when building on OS X. Created attachment 149702 [details]
Fix clang build
(In reply to comment #7) > The cleanest solution is probably to just set GTEST_HAS_RTTI=0 in GCC_PREPROCESSOR_DEFINITIONS in General.xcconfig. That'll tell gtest not to use RTTI with any of our compilers when building on OS X. I guess I can do that if you don't like my new patch. (In reply to comment #9) > (In reply to comment #7) > > The cleanest solution is probably to just set GTEST_HAS_RTTI=0 in GCC_PREPROCESSOR_DEFINITIONS in General.xcconfig. That'll tell gtest not to use RTTI with any of our compilers when building on OS X. > > I guess I can do that if you don't like my new patch. I think I'd prefer a tweak to the configuration settings than tweaking a header in a non-obvious way that would be very likely to get undone if we pull in a new version of gtest. (In reply to comment #10) > I think I'd prefer a tweak to the configuration settings than tweaking a header in a non-obvious way that would be very likely to get undone if we pull in a new version of gtest. Okay, uploading a new patch in a minute. (In reply to comment #11) > (In reply to comment #10) > > I think I'd prefer a tweak to the configuration settings than tweaking a header in a non-obvious way that would be very likely to get undone if we pull in a new version of gtest. > > Okay, uploading a new patch in a minute. Unfortunately, some don't seem to be able to seem to be seeing the variable I defined in General.xcconfig (ones in InjectedBundleTestWebKitAPI.build). Sorry, it'll also need to go in Tools/TestWebKitAPI/Configurations/Base.xcconfig, same as the setting of GTEST_HAS_TR1_TUPLE. Created attachment 149708 [details]
Patch
(In reply to comment #13) > Sorry, it'll also need to go in Tools/TestWebKitAPI/Configurations/Base.xcconfig, same as the setting of GTEST_HAS_TR1_TUPLE. Thanks for the help! Done. Committed r121331: <http://trac.webkit.org/changeset/121331> |