[PATCH] D16403: Add scope information to CFG for If/While/For/Do/Compound/CXXRangeFor statements

[Maxim Ostapenko via Phabricator via cfe-commits]

m.ostapenko updated this revision to Diff 129985.
m.ostapenko added a comment.

Hi Devin,

now I'm very sorry for a such long delay. Now I have a bunch of time to proceed development of this patch (if scope contexts are still needed, of course).
Regarding to approach you suggested (reuse LocalScope infrastructure and use first VarDecl for ScopeBegin): it seems very reasonable for me, but perhaps I need more advisory here.
I've implemented a draft patch to make sure I've understood you correctly (this is not a final version, the test case is completely inadequate for now). While testing, I've discovered several questions that I would like to discuss:

1. Do we need to have one-to-one mapping between ScopeBegins and corresponding ScopeEnds or is it OK to assume that ScopeEnd can terminate several nested scopes?
2. In the following example:

  class A {
  public:
    A() {}
    ~A() {}
    operator int() const { return 1; }
  };
  
  bool UV;
  void test_for_implicit_scope() {
    for (A a; A b = a; )
        if (UV) continue;
        A c;
    }

  it seems that lifetime of **b** ends when we branch out from the loop body (if entered, of course), but it seems that in current implementation we don't generate an implicit destructor for **b** just before **continue**. Is this a bug, or perhaps I'm missing something?
  # The approach with first VarDecl works not so well with the following test case:

  void test_goto() {
    A f;
  l0:
    A d;
    { 
      A a;
      if (UV) goto l0;
      if (UV) goto l1;
      A b;
    }
  l1:
    A c;
  }

in this approach we'll don't emit a ScopeBegin for **d** because the first VarDecl is **f**. However IMHO we still need to add ScopeBegin for **d** in order to handle d's scope end occurring when we jumping to //l0// via **if (UV) goto l0;**. I think this can be solved by adding ScopeBegin for **d** when backpatching blocks late in **buildCFG** routine (when we know all targets of all gotos). Does this approach look reasonable for you?

Thanks.


https://reviews.llvm.org/D16403

Files:
  include/clang/Analysis/AnalysisDeclContext.h
  include/clang/Analysis/CFG.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/Analysis/AnalysisDeclContext.cpp
  lib/Analysis/CFG.cpp
  lib/StaticAnalyzer/Core/AnalysisManager.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/scopes-cfg-output.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16403.129985.patch
Type: text/x-patch
Size: 53079 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180116/7c080005/attachment-0001.bin>