r175326 - Rework the visibility computation algorithm in preparation

[John McCall]

On Feb 19, 2013, at 1:07 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> Rework the visibility computation algorithm in preparation
>> for distinguishing type vs. value visibility.
>> 
>> The changes to the visibility of explicit specializations
>> are intentional.
> 
> Thanks. It is indeed much cleaner to ignore other sources when a
> specialization explicitly says what visibility should be used. A
> change that looks unintended is that in
> 
>  struct HIDDEN foo {
>  };
>  template <class P>
>  struct bar {
>    static DEFAULT void zed();
>  };
>  template <class P>
>  void bar<P>::zed() {
>  }
> 
>  //template class bar<foo>;
> 
>  void test() {
>    bar<foo>::zed();
>  }
> 
> We produce a hidden symbol for zed, but uncommenting the explicit
> template instantiation causes us to produce a default symbol. The idea
> was for explicit template instantiations to make a difference only if
> they have an attribute, no?

I agree; this should clearly produce a hidden symbol.

>> The change to the "ugly" test case is
>> a consequence of a sensible implementation, and I am happy
>> to argue that this is better behavior.
> 
> This one I think we need back :-(
> 
> The problem is that in gcc it is really easy for a project to use
> -fvisibility=hidden. They change the build system and then add default
> visibility attributes to *definitions* that they need to show up in
> the libraries. That is how we end up with things like:
> 
>  template<typename T>
>  struct DEFAULT foo {
>    void bar() {}
>  };
>  class zed;
>  template class foo<zed>;
>  class DEFAULT zed {
>  };


> The bit I think we need back from the old implementation is that an
> implicit visibility, like the one zed has up to its definition, should
> not force foo<zed> to have a hidden visibility since foo has an
> explicit visibility. This does mean we would produce a default symbol
> for

The purpose of -fvisibility=hidden is to allow people to hide all of their
declarations without explicitly putting HIDDEN everywhere.  There's no
reason that shouldn't propagate through templates.  Doing so would
really screw up libc++, which puts DEFAULT on all its templates so that
type matching will work across dylib boundaries.

> It was originally found by clang being unable to build chrome in
> "components mode". I did try making it an error, but that then
> requires changing quiet a bit of code to forward declare types with
> the correct visibility or avoid the forward declaration.

Chrome is relying on a quirk of GCC's translation model here.
Clang generally emits LLVM IR while it's still processing the file, which
means doing the visibility computation straight off.  GCC doesn't even
emit GIMPLE until it's done parsing the entire file, so the visibility
computation necessarily has all possible information.  There is no
way to slightly tweak Clang's visibility model to match the effect of what
GCC does — you'd need to either delay all code emission until the end
of the file, or at least recompute visibility for everything then.

But I still don't understand the problem that you're saying Chrome has.
Clang actually *does* delay the emission of template members until
the end of the file by default, because we don't even *instantiate* them
until then.  The only time we'd emit them in the middle of a file is if you
explicitly instantiated them.  Why is it difficult for Chrome to ensure that
template arguments are all properly annotated/defined before
performing explicit instantiations on them?  How many explicit
instantiations can you possibly have?

John.