r285647 - [index] Fix repeated visitation of the same InitListExpr for indexing.

[Argyrios Kyrtzidis via cfe-commits]

> On Oct 31, 2016, at 4:49 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Mon, Oct 31, 2016 at 3:12 PM, Argyrios Kyrtzidis via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> Author: akirtzidis
> Date: Mon Oct 31 17:12:12 2016
> New Revision: 285647
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=285647&view=rev <http://llvm.org/viewvc/llvm-project?rev=285647&view=rev>
> Log:
> [index] Fix repeated visitation of the same InitListExpr for indexing.
> 
> It was visited multiple times unnecessarily.
> 
> rdar://28985038
> 
> Added:
>     cfe/trunk/test/Index/Core/designated-inits.c
> Modified:
>     cfe/trunk/lib/Index/IndexBody.cpp
> 
> Modified: cfe/trunk/lib/Index/IndexBody.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexBody.cpp?rev=285647&r1=285646&r2=285647&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexBody.cpp?rev=285647&r1=285646&r2=285647&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/Index/IndexBody.cpp (original)
> +++ cfe/trunk/lib/Index/IndexBody.cpp Mon Oct 31 17:12:12 2016
> @@ -300,6 +300,7 @@ public:
>        IndexingContext &IndexCtx;
>        const NamedDecl *Parent;
>        const DeclContext *ParentDC;
> +      bool Visited = false;
> 
>      public:
>        SyntacticFormIndexer(IndexingContext &indexCtx,
> 
> Do we need SyntacticFromIndexer to be a RecursiveASTVisitor at all? It looks like you would get the desired effect by simply walking the elements of the syntactic form looking for DesignatedInitExprs (which will always be at the top level of the InitListExpr's element if they exist). Right now, it looks like you still traverse expression E twice in cases like this:
> 
>   int arr[] = { [0] = 0, E };
> 
> ... and you'd still get a quadratic traversal for a case like { [0] = {}, { [0] = {}, { [0] = {}, ... } } }.

This is excellent feedback, see r285666.
Thanks for reviewing!

>  
> @@ -308,6 +309,22 @@ public:
> 
>        bool shouldWalkTypesOfTypeLocs() const { return false; }
> 
> +      bool TraverseInitListExpr(InitListExpr *S, DataRecursionQueue *Q = nullptr) {
> +        // Don't visit nested InitListExprs, this visitor will be called again
> +        // later on for the nested ones.
> +        if (Visited)
> +          return true;
> +        Visited = true;
> +        InitListExpr *SyntaxForm = S->isSemanticForm() ? S->getSyntacticForm() : S;
> +        if (SyntaxForm) {
> +          for (Stmt *SubStmt : SyntaxForm->children()) {
> +            if (!TraverseStmt(SubStmt, Q))
> +              return false;
> +          }
> +        }
> +        return true;
> +      }
> +
>        bool VisitDesignatedInitExpr(DesignatedInitExpr *E) {
>          for (DesignatedInitExpr::Designator &D : llvm::reverse(E->designators())) {
>            if (D.isFieldDesignator())
> 
> Added: cfe/trunk/test/Index/Core/designated-inits.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/designated-inits.c?rev=285647&view=auto <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/designated-inits.c?rev=285647&view=auto>
> ==============================================================================
> --- cfe/trunk/test/Index/Core/designated-inits.c (added)
> +++ cfe/trunk/test/Index/Core/designated-inits.c Mon Oct 31 17:12:12 2016
> @@ -0,0 +1,33 @@
> +// RUN: c-index-test core -print-source-symbols -- %s -target x86_64-apple-macosx10.7 | FileCheck %s
> +
> +struct MyStruct {
> +  int myfield;
> +};
> +
> +enum {
> +  MyValToSet;
> +};
> +
> +// CHECK: [[@LINE+1]]:14 | struct/C | MyStruct |
> +const struct MyStruct _MyStruct[]
> +  [16]
> +  [3]
> +  [3]
> +  [2]
> +  [2] = {
> + [0] = {
> +    [0] = {
> +      [0] = {
> +        [0][0] = {
> +          [0] = {
> +            // CHECK: [[@LINE+2]]:14 | field/C | myfield | {{.*}} | Ref |
> +            // CHECK: [[@LINE+1]]:24 | enumerator/C | MyValToSet |
> +            .myfield = MyValToSet,
> +            // CHECK-NOT: | field/C | myfield |
> +            // CHECK-NOT: | enumerator/C | MyValToSet |
> +          },
> +        },
> +      },
> +    },
> +  },
> +};
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161031/3b06c7c1/attachment-0001.html>