[PATCH] D43428: [CFG] NFC: Allow more complicated construction contexts.

Fri Feb 16 19:27:15 PST 2018

NoQ created this revision.
NoQ added reviewers: rsmith, dcoughlin, xazax.hun, a.sidorin.
Herald added subscribers: cfe-commits, rnkovacs, kristof.beyls, mehdi_amini.

`ConstructionContext`s introduced in https://reviews.llvm.org/D42672 are an additional piece of information included with `CFGConstructor` elements that help the client of the CFG (specifically, the static analyzer - it's off for other clients)  to understand where the newly constructed object is stored. For now `ConstructionContext` was only capable of containing a single statement, such as a `DeclStmt` (if it's a variable's constructor) or a `CXXNewExpr` (if it's a heap object) or a `CXXCtorInitializer` (if it's a field or a base-class instance - this isn't a statement however) or a `CXXBindTemporaryExpr` (if it's a temporary that requires a destructor call).

This patch continues the effort as planned. In order to handle more complex constructions, richer construction contexts are required. This patch adds two new possibilities (without immediately using any of them):

- to nest construction contexts into each other, as a linked list that is constructed gradually during AST traversal;
- to maintain construction contexts for multiple constructors simultaneously.

I'd give an example of how it's planned to be used here, and post a separate revision with actually using them that way. Consider the ternary operator example from http://lists.llvm.org/pipermail/cfe-dev/2018-February/056898.html:

  1    class C {
  2    public:
  3      C(int) {}
  4      ~C() {}
  5    };
  6
  7    void foo(int coin) {
  8      const C &c = coin ? C(1) : C(2);
  9    }

  DeclStmt 0x7fc1e20023e0 <line:8:3, col:34>
  `-VarDecl 0x7fc1e2001dc8 <col:3, col:33> col:12 c 'const C &' cinit
    `-ExprWithCleanups 0x7fc1e2002370 <col:16, col:33> 'const C' lvalue
      `-MaterializeTemporaryExpr 0x7fc1e2002340 <col:16, col:33> 'const C' lvalue extended by Var 0x7fc1e2001dc8 'c' 'const C &'
        `-ImplicitCastExpr 0x7fc1e2002328 <col:16, col:33> 'const C' <NoOp>
          `-ConditionalOperator 0x7fc1e20022f8 <col:16, col:33> 'C'
            |-ImplicitCastExpr 0x7fc1e2002170 <col:16> 'bool' <IntegralToBoolean>
            | `-ImplicitCastExpr 0x7fc1e2002158 <col:16> 'int' <LValueToRValue>
            |   `-DeclRefExpr 0x7fc1e2001e28 <col:16> 'int' lvalue ParmVar 0x7fc1e2001c18 'coin' 'int'
            |-CXXBindTemporaryExpr 0x7fc1e2002248 <col:23, col:26> 'C' (CXXTemporary 0x7fc1e2002240)
            | `-CXXConstructExpr 0x7fc1e2002208 <col:23, col:26> 'C' 'void (const C &) noexcept' elidable
            |   `-MaterializeTemporaryExpr 0x7fc1e20021a0 <col:23, col:26> 'const C' lvalue
            |     `-ImplicitCastExpr 0x7fc1e2002188 <col:23, col:26> 'const C' <NoOp>
            |       `-CXXFunctionalCastExpr 0x7fc1e2002078 <col:23, col:26> 'C' functional cast to class C <ConstructorConversion>
            |         `-CXXBindTemporaryExpr 0x7fc1e2002058 <col:23, col:26> 'C' (CXXTemporary 0x7fc1e2002050)
            |           `-CXXConstructExpr 0x7fc1e2002018 <col:23, col:26> 'C' 'void (int)'
            |             `-IntegerLiteral 0x7fc1e2001e60 <col:25> 'int' 1
            `-CXXBindTemporaryExpr 0x7fc1e20022d8 <col:30, col:33> 'C' (CXXTemporary 0x7fc1e20022d0)
              `-CXXConstructExpr 0x7fc1e2002298 <col:30, col:33> 'C' 'void (const C &) noexcept' elidable
                `-MaterializeTemporaryExpr 0x7fc1e2002280 <col:30, col:33> 'const C' lvalue
                  `-ImplicitCastExpr 0x7fc1e2002268 <col:30, col:33> 'const C' <NoOp>
                    `-CXXFunctionalCastExpr 0x7fc1e2002130 <col:30, col:33> 'C' functional cast to class C <ConstructorConversion>
                      `-CXXBindTemporaryExpr 0x7fc1e2002110 <col:30, col:33> 'C' (CXXTemporary 0x7fc1e2002108)
                        `-CXXConstructExpr 0x7fc1e20020d0 <col:30, col:33> 'C' 'void (int)'
                          `-IntegerLiteral 0x7fc1e20020b0 <col:32> 'int' 2 

Look at the two elidable copy-constructors. As explained in the aforementioned mail, if we want the CFG client to understand how lifetime extension works, we need to include both the surrounding `CXXBindTemporaryExpr` (`0x7fc1e2002248` for the first constructor and `0x7fc1e20022d8` for the second constructor) and the surrounding `MaterializeTemporaryExpr` (`0x7fc1e2002340` for both constructors) in the construction context.

While traversing the AST in `findConstructionContexts()` (formerly known as `EnterConstructionContextIfNecessary()`), we'll first encounter `MaterializeTemporaryExpr` `0x7fc1e2002340`. We'll create a `ConstructionContext` for `MaterializeTemporaryExpr` and set it as the current `ContextSoFar`. Then we'll encounter the first `CXXBindTemporaryExpr` and change `ContextSoFar` to the new context that has `CXXBindTemporaryExpr` as the trigger statement and the old context as its parent. Then we'll see the constructor within the `CXXBindTemporaryExpr` and realize that we've just built its construction context. Then we'd pop out of the `CXXBindTemporaryExpr` and restore the old `MaterializeTemporaryExpr`-based context and go to another branch of our `ConditionalOperator`. Here we'd see the other `CXXBindTemporaryExpr`, for which we'd do the same thing. With that, both constructors would have their contexts set (simultaneously!), and their contexts would be triggered by their respective `CXXBindTemporaryExpr` and "previously triggered" (through the parent context) by their common `MaterializeTemporaryExpr`. Then during normal visit, we'd encounter both constructors and add their CFG elements with their respective construction contexts.

Note that later when we visit these `CXXBindTemporaryExpr`s during normal visit, we'd have to try to see if they form construction contexts for any of their children, which would mean that we'd try to create a construction context for the same constructor twice. This makes it a bit tricky to assert that the algorithm is working correctly; previously we could assert that the construction context is not being set twice. Now we make a weaker assertion, but still a powerful one: when we're trying to set the construction context twice, on our second attempt we should be having a construction context that is a sub-context of the context that we already have. In this case we keep the old context because it provides more informantion.

I also added a new assertion that says that all construction contexts were fully consumed when CFG is fully built.

A similar process is planned to be used for aggregate constructors, where the context may be identified via a chain of multiple `InitListExpr`s (which may also contain multiple constructors).

Because the `ConstructionContext` class is becoming increasingly complex, i strongly believe i should split it up into sub-classes enumerated by kinds.

Repository:
  rC Clang

https://reviews.llvm.org/D43428

Files:
  include/clang/Analysis/CFG.h
  lib/Analysis/CFG.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43428.134780.patch
Type: text/x-patch
Size: 9663 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180217/4a4a4d3e/attachment.bin>