[PATCH] D67079: [analyzer] CastValueChecker: Model inheritance

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:174
+
+constexpr llvm::StringLiteral Vowels = "aeiou";
+
----------------
Charusso wrote:
> NoQ wrote:
> > Omg lol nice. Did you try to figure out how do other people normally do it?
> There is no function for that in `ADT/StringExtras.h` + `grep` did not help, so I realized it is a common way to match vowels. Do you know a better solution?
I just realized that this is actually incorrect and the correct solution is pretty hard to implement because the actual "//a// vs. //an//" rule deals with sounds, not letters. Eg.: 

> Clang is an "LLVM native" C/C++/Objective-C compiler

has an "an" because it's read as "an //**e**//l-el-vee-am".


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:136
+          CurrentBase.getType()->getAsCXXRecordDecl())
+        CurrentBaseFoundPreviously = true;
+    }
----------------
Using a flag here is unnecessary because you're returning true without doing anything else whenever the flag is set.

So, basically, your code says "if at least one current base is different from at least one previous base, the cast succeeds".

In particular, this function would return true whenever `CurrentRD` and `PreviousRD` are from //completely unrelated// class hierarchies. It sounds like whatever `isSucceededDowncast()` was supposed to do, it's not doing it right.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67079/new/

https://reviews.llvm.org/D67079