r175796 - Replace CFGElement llvm::cast support to be well-defined.

[David Blaikie]

On Fri, Feb 22, 2013 at 4:08 PM, Ted Kremenek <kremenek at apple.com> wrote:

> On Feb 22, 2013, at 9:53 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
> My only concern is that there's already a sense of invalidity for
> CFGElement and TypeLocs that's publicly usable, so now the APIs would be
> returning Optional<T> where T is (without any type safety helping in this
> case) guaranteed to be "not invalid".
>
>
> That's my main concern. If you have a type that has an invalid state, you
> have to be aware when objects of that type has invalid state. If we then
> wrap an Optional<> around it, now you have two axes of invalid state to
> worry about… which is actively confusing. So, for types that already have
> an invalid state, I'd rather not have getAs<> return an Optional<> because
> we'd end up with the two axes of invalidity. For types that *don't* have an
> invalid state, returning Optional<> from getAs<> would be a great solution.
>
> And if we can eliminate the invalid states for some types, switching over
> to Optional<> for the places where do getAs<> and similar operations, I
> think that would improve the clarity of the code base.
>
> Excellent - fixed CFG* hierarchy to remove the invalid state & use
Optional<CFG*> instead in r175938.

I might consider what it'll take to transform TypeLoc in a similar way at
some point in the future.


> Right.  I also think "invalid" has different meanings for some of our
> types.
>

Yep, it's by no means a global solution & it wasn't obvious to me which
'invalid' states were invalid enough to remove/replace with Optional. We'll
take them on a case-by-case basis.


> For example, for SourceLocation "invalid" means that there is no location,
> but that still is a legal location that can be *used*.  In the case of
> CFGStmt, and invalid CFGStmt is one that can't be used at all; it's
> tantamount to a dangling pointer.
>
> Another analogy is the difference between "the empty set" and "no set".
>  They are completely different concepts.  I think in our codebase the term
> "validity" is sometimes used for one or the other.  For me, Optional<> maps
> to "no set" or "there is a set".
>
>
> So here's a hypothetical step further: along with specializing Optional to
> use zero-cost "optionality" (I suppose that would negate the concern I have
> above - essentially Optional<T> would be another representation for
> "invalid T"), make it so that no code deals with raw invalid Ts (the only
> place where invalid Ts could be constructed would be in Optional, via
> friending). That way we get zero cost type safe invalid states.
>
>
> Making Optional<T> have zero space cost when possible would be a great
> optimization. I'd consider that orthogonal to the design discussion,
> because I don't think we're motivated either way by the current cost of
> Optional.
>
>
> Agreed.
>

Fair point - I've left that as an exercise for the future (for me or
someone else). None of the Optional<CFG*> were members so the extra space
consumption is only a minor stack issue by the looks of it.

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130222/01a317d3/attachment.html>