This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] Fix invalidation on C++ const methods.
ClosedPublic

Authored by NoQ on Jun 21 2018, 3:15 PM.

Details

Summary

const methods shouldn't invalidate the object unless mutable fields kick in.

They sometimes were invalidating the object when we accidentally failed to retrieve the record type to see if there are mutable fields in it.

We failed to retrieve it because this-expression is sometimes of pointer type and sometimes of object type, and we only handled the latter.

Diff Detail

Repository
rC Clang

Event Timeline

NoQ created this revision.Jun 21 2018, 3:15 PM

I find it a strange behavior that this-expression is sometimes a pointer, sometimes a record declaration. If references are resolved, why are pointers not?

This is an important fix, but I wonder how many other places are in the code where we do not handle this-expressions by pointers.

NoQ added a comment.Jun 23 2018, 3:58 PM

Well, i guess that's just one of those mildly infuriating aspects of the AST and/or the standard :)

If references are resolved, why are pointers not?

It's kinda understandable. References are simple aliases to glvalues. When you use a reference, you simply get *the* original glvalue, which is exactly what the AST gives you: a glvalue expression of object type. Here's what Expr::setType() says:

130   void setType(QualType t) {
131     // In C++, the type of an expression is always adjusted so that it
132     // will not have reference type (C++ [expr]p6). Use
133     // QualType::getNonReferenceType() to retrieve the non-reference
134     // type. Additionally, inspect Expr::isLvalue to determine whether
135     // an expression that is adjusted in this manner should be
136     // considered an lvalue.
137     assert((t.isNull() || !t->isReferenceType()) &&
138            "Expressions can't have reference type");
139
140     TR = t;
141   }

This code is still imperfect because it doesn't take dynamic type of the region into account, which may be more specific than the expression's type. I guess i'll add a FIXME.

This revision is now accepted and ready to land.Jun 24 2018, 9:58 AM
This revision was automatically updated to reflect the committed changes.