Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

RecursiveASTVisitor: VisitTypeOfExprType doesn't get called #13662

Open Quuxplusone opened 12 years ago

Quuxplusone commented 12 years ago
Bugzilla Link PR13618
Status NEW
Importance P normal
Reported by Sergejs (sergejs.belajevs@gmail.com)
Reported on 2012-08-16 09:29:48 -0700
Last modified on 2012-08-17 16:28:17 -0700
Version trunk
Hardware PC Windows NT
CC klimek@google.com, llvm-bugs@lists.llvm.org, philipjcraig@gmail.com, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
I wrote this simple recursive AST visitor:

class MyVisitor : public clang::RecursiveASTVisitor<MyVisitor>
{
public:
    MyVisitor() {}

    bool VisitTypeOfExprType(clang::TypeOfExprType* t)
    {
        printf("Visiting typeof expr type!\n");
        return true;
    }
};

And ran it on AST of the following code (found in one open-source project):

zbx.c:

void foo()
{
  int status = 0;

  status = ((((__extension__ (((union { __typeof(status) __in; int __i; }) { .__in = (status) }).__i))) & 0xff00) >> 8);

  status = ({ union { __typeof(status) __in; int __i; } a; a.__in = status; (a.__i & 0xff00) >> 8; });
}

I got only one "Visiting typeof expr type!" as a result, for the 2nd anonymous
union. I cannot find a way how to catch __typeof of the first anonymous union
with RAV.
Quuxplusone commented 12 years ago
Here's a version of your visitor that can traverse the anonymous union:

class MyVisitor : public clang::RecursiveASTVisitor<MyVisitor>
{
    typedef RecursiveASTVisitor<MyVisitor> VisitorBase;
public:
    MyVisitor() {}

    bool VisitTypeOfExprType(clang::TypeOfExprType* t)
    {
        printf("Visiting typeof expr type!\n");
        return true;
    }

    bool TraverseCompoundLiteralExpr(CompoundLiteralExpr *E) {
        VisitorBase::TraverseCompoundLiteralExpr(E);
        TraverseTypeLoc(E->getTypeSourceInfo()->getTypeLoc());
        return true;
    }

    bool VisitRecordType(RecordType *T) {
        RecordDecl *D = T->getDecl();
        if (!D->getIdentifier())
          TraverseDecl(D);
        return true;
    }
};

The CompoundLiteralExpr change would be straight forward to add to RAV.

I'm not sure about the RecordType change though. There's two problems.

First, I don't know whether getIdentifier() is the correct test. I expected
isAnonymousStructOrUnion() to be a better test but it didn't work.

Second, this changes results in the 2nd anonymous union being traversed twice.
This is because the declaration of 'a' is parsed into a DeclStmt containing
both a RecordDecl and a VarDecl (and there is a RecordType as part of the
VarDecl), whereas I would have expected only the VarDecl.
Quuxplusone commented 12 years ago

Thanks, this workaround seems to work, all tests from different projects pass now!

Sergejs

Quuxplusone commented 12 years ago
> The CompoundLiteralExpr change would be straight forward to add to RAV.

I agree, fixed in r162133.

> I'm not sure about the RecordType change though.

Right, that change is not correct (for instance, a named type can be defined in
a compound literal in C). I *think* we want two changes here:

* Avoid repeatedly visiting the decl-specifier-seq when visiting a DeclStmt, and
* Visit the TagDecl when we visit a TagTypeLoc where isDefinition() returns
true.