On Sun, Apr 22, 2012 at 1:40 PM, Nico Weber <[email protected]> wrote: > Hi Rafael, > > I tried building chrome with the components build with both trunk @ > r155304 and your patch.
Sorry, make that trunk @ r155317. 155304 is my llvm revision number 9_9 tests-MacBook-Pro-2:clang test$ ../../Release+Asserts/bin/clang --version clang version 3.2 (trunk 155317) (llvm/trunk 155304) > On trunk, all targets build fine. With this > patch, target 'ipc' builds correctly (this was broken before your > revert), so that's good. Target 'net' however fails to link: > > Undefined symbols for architecture i386: > "std::string* logging::MakeCheckOpString<std::string, > std::string>(std::string const&, std::string const&, char const*)", > referenced from: > > net::HostResolverImpl::Job::AddRequest(scoped_ptr<net::HostResolverImpl::Request>) > in net.host_resolver_impl.o > net::HostResolverImpl::Job::CancelRequest(net::HostResolverImpl::Request*) > in net.host_resolver_impl.o > net::MimeUtil::MatchesMimeType(std::string const&, std::string > const&) const in net.mime_util.o > net::CookieMonster::TrimDuplicateCookiesForKey(std::string > const&, std::_Rb_tree_iterator<std::pair<std::string const, > net::CookieMonster::CanonicalCookie*> >, > std::_Rb_tree_iterator<std::pair<std::string const, > net::CookieMonster::CanonicalCookie*> >) in net.cookie_monster.o > net::HttpAuthCache::Add(GURL const&, std::string const&, > net::HttpAuth::Scheme, std::string const&, net::AuthCredentials > const&, std::string const&) in net.http_auth_cache.o > ld: symbol(s) not found for architecture i386 > > Thanks for looking at this :-) > > Nico > > > ps: I noticed that your patch doesn't add a new test case for whatever > is different for target 'ipc'. If you want, I can try to build a > reduced test case of what went wrong before your revert and check that > in. I can do this for the problem mentioned in this mail too. (Will > take until tomorrow though.) > > pps: Sorry, didn't Reply All the first time round. > > On Sun, Apr 22, 2012 at 11:49 AM, Rafael Espíndola > <[email protected]> wrote: >> Now with the patch :-( >> >> >> 2012/4/22 Rafael Espíndola <[email protected]>: >>> Given >>> >>> namespace t1 { >>> template<typename T> >>> class DEFAULT foo { >>> void bar() {} >>> }; >>> struct HIDDEN zed { >>> }; >>> template class DEFAULT foo<zed>; >>> } >>> namespace t2 { >>> template<typename T> >>> class DEFAULT foo { >>> void bar() {} >>> }; >>> struct HIDDEN zed { >>> }; >>> template class foo<zed>; >>> } >>> >>> gcc 4.7 produces a default symbol for the first test and a hidden one >>> for the second one. This requires differentiating attributes in a >>> template and attributes in an instantiation. Clang currently doesn't >>> do that for any attributes, but Jeff tells me that "C++11 just says >>> that attributes appertain to particular things, not how they propagate >>> from templates to instantiations" so this looks like a valid c++ >>> extension by gcc. >>> >>> The attached patch: >>> >>> * Avoids cloning the visibility attribute during instantiations. >>> * Avoids considering visibility info in a template when the >>> instantiation has an attribute. This lets us get the first testcase >>> right. >>> * Goes back to selecting the minimum visibility in cases where we >>> should look at the template visibility. This lets us get the second >>> testcase right. >>> >>> The summary is that this fixes cases where where we were being >>> unnecessary conservative in producing a default visibility but avoids >>> breaking chrome's build. >>> >>> Cheers, >>> Rafael _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
