[PATCH] D18635: Rework interface for bitset-using features to use a notion of class scope.

Peter Collingbourne via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 31 17:30:07 PDT 2016


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





More information about the cfe-commits mailing list