pcc added inline comments.
================
Comment at: docs/ClassScope.rst:2
@@ +1,3 @@
+===========
+Class Scope
+===========
----------------
rsmith wrote:
> pcc wrote:
> > 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.
> I'm really not happy about having a third granularity for entity uniqueness
> (LTO granularity) between DSO granularity (visibility) and module granularity
> (linkage). But if we need that to accurately represent the state of the
> world, then perhaps there's no way to escape it (although if that is the
> intent, then the documentation here should be talking about something like
> LTO units rather than linkage units).
>
> 1) Would it be inappropriate for entities in `namespace std` on Windows to
> be given default visibility when people do this?
>
> 2) I don't think it makes sense for the `-fsanitize=cfi*` flag to enable
> "types can only be declared within their own LTO unit by default" semantics,
> whether those semantics are related to devirtualization, CFI, or symbol
> visibility. Sanitizer flags are supposed to turn on checks for the ambient
> configuration, not change what the code means; I'd prefer to say that it's an
> error if you enable `-fsanitize=cfi*` and don't *also* enable
> `fvisibility=hidden` (or a hypothetical `-fclass-visibility=hidden`).
1. Deciding to treat `std` specially based on flags like `/MT` makes sense to
me, but visibility isn't a thing on Windows, and dll*port affects the ABI, so
we probably need the in-between state there.
2. Okay, I prefer that over what I proposed. I suppose users could specify
`-fvisibility=default` if that's really what they want.
Let's say we simplify this down to no new driver flags and one new attribute
that disables vtable opt and CFI on hidden visibility and non-dll*port classes
(no namespace attributes). Let me confirm that that's enough for the use cases
I care about.
http://reviews.llvm.org/D18635
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
