[cfe-dev] Fwd: CFGElement changes and initializers addition (with patch)

[Marcin Świderski]

Sorry, I still forget to choose "Replay all" option...

W dniu 20 sierpnia 2010 05:35 użytkownik Ted Kremenek
<kremenek at apple.com>napisał:


> On Aug 18, 2010, at 12:11 AM, Marcin Świderski wrote:
>
> Hi Jordy, Ted
>
> Thanks for the appreciation and for comments. Some comments inline:
>
> W dniu 18 sierpnia 2010 07:47 użytkownik Jordy Rose <jediknil at belkadan.com
> > napisał:
>
>>
>> Wow, good work.
>>
>> I agree with most of what Ted said. A few additional comments:
>>
>> - I like the single enum rather than kind and subkind, but there are 7
>> elements, not 6. So it won't fit in a PointerIntPair anyway, though having
>> a Stmt* field (or void*) and an enum field would be preferable to two
>> PointerIntPairs.
>> I'm still not totally convinced we need scope markers in the CFG, though.
>>
>> For both destructor kinds I will need space for two pointers. For
> automatic object I need it's declaration and statement that cause destructor
> to be called. For temporary object I need expression bound to the temporary
> and the full expression that contains the temporary.
>
>
> Repeating what I said in my reply to Jordy, I think it's fine to burn an
> extra pointer if it gives us flexibility to track more information that
> would be useful for diagnostics.
>
> So I have three acceptable solutions.
1. Two pointers for each element.
2. One pointer for each element, but in case of destructor it points to
allocated pair of pointers.
3. One pointer for each element, and location of call to destructor is
calculated on demand.

1 is wasteful, because only destructors need two pointers. 3 will give as
possibly unclean API for clients. 2 seams to be the best and I will
implement it.

>
> I could allocate separate struct for those and point to it from CFGElement.
> If we drop the scopes and move information about kind of destructor to this
> struct, then CFGElement will need only one PointerIntPair.
>
>
> That is also a possibility.  It could be allocated from the
> BumpPtrAllocator.
>
> When will be the memory allocated with BumpPtrAllocator freed? If I will
allocate memory for use in one instance of CFG, can I forget about it (not
call Deallocate)?

>
> If we would want scopes modeled we could use same struct as for
> destructors. So we would end up with something like this:
>
> - Statement and LValStatement with pointer to Stmt*,
> - Initializer with pointer to CXXBaseOrMemberInitializer*,
> - Extendend CFGElement with pointer to struct with information about
> destructor or scope.
>
> Our kind enum would look like this:
> enum Kind {
>   Statement,
>   LvalStatment,
>   BEGIN_STATEMENT = Statement,
>   END_STATEMENT = LvalStatement,
>   Initializer,
>
>   ExtendedElement, // This would be internal, client would never get this
>                               // with getKind() method.
>
>   AutomaticObjectDtor,
>   TemporaryObjectDtor,
>   BEGIN_DTOR = AutomaticObjectDtor,
>   END_DTOR = TemporaryObjectDtor,
>   StartScope,
>   EndScope,
>   BEGIN_SCOPE = StartScope,
>   END_SCOPE = EndScope
> };
>
>
> What is the purpose of the 'ExtendedElement' kind?
>
> Int part of PointerIntPair where pointer part points to pointer can have
only 2 bits. ExtendedElement kind marks that the kind (-
int(ExtendedElement)) is stored in int part of first PointerIntPair in
pointed struct. It is an implementation detail realy.

>
> This way we will have space efficient CFG for Obj-C and C languages. For
> C++ CFG will occupy less memory if there will be more statement then
> destructor CFGElements.
>
> - Why don't we make this a static type hierarchy like SVal or MemRegion?
>> We'd have CFGStmt, CFGInitializer, CFGDestructor, and CFGScope(Marker)
>> inheriting from CFGElement. That way we can have, among other things, a
>> CFGStmt that simplifies to a Stmt. Instead of asserts, we can just have
>> the
>> type-specific methods on the subclasses.
>>
>> Yes, this will be much better approach. I will change this.
>
>
> Sounds great.
>
>
The hierarchy is almost ready. I'll send a patch with this and both
destructors implemented after weekend.

>
>

> - In UnreachableCodeChecker::getUnreachableStmt(), isn't the block
>> guaranteed to have an initial statement for the same reason as in
>> ReachableCode.cpp?
>>
>> Do you mean that if block is unreachable it will start with a statement
> and not with initializer, destructor or scope?
>
>
> Yes, an unreachable block could certainly start with an initializer if it
> had no initializer argument and the previous initializer called a no-return
> function.  Gross, but I guess possible.
>
> Moreover, a call to builtin_unreachable is not guaranteed to be the first
> CFGElement in a CFGBlock.  I think the checker's logic is likely wrong here.
>  It should be iterating over the block, looking for this specific CallExpr.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100820/b46960c1/attachment.html>