[cfe-commits] [patch] Template instantiation and visibility

[Nico Weber]

Hi Rafael,

I tried building chrome with the components build with both trunk @
r155304 and your patch. 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
<rafael.espindola at gmail.com> wrote:
> Now with the patch :-(
>
>
> 2012/4/22 Rafael Espíndola <rafael.espindola at gmail.com>:
>> 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