pcc added inline comments.

================
Comment at: docs/ClassScope.rst:2
@@ +1,3 @@
+===========
+Class Scope
+===========
----------------
rsmith wrote:
> pcc wrote:
> > rsmith wrote:
> > > Can you use some word other than "scope" here? "Class scope" is already a 
> > > term of art in C++, meaning something completely different. I think what 
> > > you're referring to is exactly the visibility of the class (in the ELF 
> > > sense).
> > Yes, this is pretty much visibility. I wanted to avoid using the term 
> > "visibility" because I'm introducing flags and attributes which can make 
> > scope mean something different to object file visibility, so I wanted to 
> > avoid the overload to avoid confusion.
> > 
> > Maybe the overloading would help with understanding though if I add a 
> > qualifying adjective. This is all about whether all derived classes are 
> > visible, so maybe the right term is something like "derived visibility"?
> We already have attributes that can set the visibility of a class (which in 
> turn affects the visibility of the vtable etc.) In what way is that different 
> from what you're proposing? Is this a valuable difference, given the 
> complexity of having two similar-but-different ways of describing the 
> cross-DSO visibility of a class?
Yes, ideally I'd like to just use visibility for this. There were a couple of 
things that motivated a design that separated the concepts, though:

1. The part of the executable or DSO that we're LTOing may be a subset of the 
whole executable or DSO. If the user has prebuilt object files that they need 
to link into their executable or DSO, we need to exclude those classes from our 
analysis. One example of this would be the standard library. On most platforms 
this is marked up with default visibility attributes, so we're fine, but on 
Windows the platform ships an un-marked up standard library as a static 
library, and allows users to link to it. The idea is that Windows users who use 
the static standard library would be able to say
```
namespace [[clang::global_scope]] std {}
```
in a `-include`d header and get global scope on the standard library.

2. I wanted `-fsanitize=cfi*` to turn on CFI checks on classes by default. It 
would be surprising to users if that flag on its own did not enable CFI on 
classes declared in the normal way, and it could easily lead to users shipping 
unprotected binaries. I suppose an alternative thing we could do would be to 
make `-fsanitize=cfi*` imply `-fvisibility=hidden`. Then if things break, they 
would at least be likely to break loudly.


http://reviews.llvm.org/D18635



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to