[llvm] r256620 - [ptr-traits] Implement the base pointer traits using the actual

[Chandler Carruth via llvm-commits]

Ah, thanks, you got to it before I did. I'll test everything, check your
patches, and re-land!

I also (now that I'm using a real keyboard instead of a phone) wanted to
more fully respond to your concerns about incomplete types. I think this is
an important point, so its worth being thorough in the examination of the
options here.

The reason to use an incomplete type, much as you indicate, is as some form
of abstraction boundary: specifically, not relying on *any aspect* of how
that type is implemented, merely that it is a type. Currently, the only
thing you can rely on is that it isn't an enum or a primitive type. I
actually think it would be great if the language even hid *that* detail
from a forward declaration. They should (IMO) work very much like "typename
T". You know its a type, but that is all.

The question becomes, is it reasonable to use such a construct in a
PointerIntPair (and consequentially in a PointerUnion or a DenseMap pointer
key).

I would argue that the only thing reasonable to do with such a type in a
PointerIntPair is to declare the PointerIntPair. The data layout and size
of such a PointerIntPair is unambiguous and clear. Similarly, it is
reasonable to declare a pointer.

But I think it is not reasonable to *use* the PointerIntPair any more than
it is reasonable to dereference the pointer. Unfortunately, use of the
PointerIntPair happens even when you assign, copy, or move it. But this is
inherent in the nature of the beast: in order to know how to behave,
PointerIntPair must know how the pointee type is *implemented*. So you can
have a member, but you will need to define your methods that operate on it
out-of-line and provide the full definition. This still allows circular
references and other common patterns, but it is more restrictive than a raw
pointer. I think that is, again, in the nature of the beast. We are
fundamentally relying on more than just the fact that it is a pointer -- we
rely on the particular alignment that pointer will always have.

The example that fully convinced me of all of this goes as follows. When
you forward declare a class, all uses of the forward declared class should
be reasonable for *any and all possible implementations* of that class.
There should never be a dependence on how the class implemented. But with
PointerIntPair, if I replace the implementation of a class with, for
example, a single unsigned char that is an index into a single table of all
255 possible instances, essentially forming a *very* compressed pimpl, it
would actually break anyone using that class in a PointerIntPair. And it
would do so silently in many cases! For me, this makes it really clear that
PointerIntPair is actually depending on a particular implementation, and
can't correctly work with an incomplete type.

Hope this helps explain my reasoning, and thanks again for the very quick
polly fixes, and apologies again for not getting polly fixed first.
-Chandler

On Wed, Dec 30, 2015 at 12:31 PM Tobias Grosser <tobias at grosser.es> wrote:

> On 12/30/2015 07:29 PM, Chandler Carruth wrote:
> > The right solution is to not use incomplete types here as all of my
> > other commits indicated. Sorry I didn't catch the uses in Polly last
> > night. They just need to be fixed like every other LLVM project. I'll
> > finish this morning and re-land.
>
> OK. I committed in r256649 and r256650 fixes for the two occurrences in
> Polly. To my understanding, it should now be save to re-land your
> commit. Regarding my Polly changes, I would appreciate a post-commit
> review.
>
> I left commit r256620 for you to re-apply, but can easily add it back if
> you prefer.
>
> Best,
> Tobias
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151230/c1d1043a/attachment.html>