Charusso added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:126
+
+ // If the casts have a common anchestor it could not be a succeeded downcast.
+ for (const auto &PreviousBase : PreviousRD->bases())
----------------
NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Counterexample:
> > >
> > > ```
> > > struct A {};
> > > struct B : A {};
> > > struct C : A, B {};
> > > ```
> > >
> > > Downcast from `C` to `B` should succeed, even though they have a common
> > > ancestor `A` (which has the same `CXXRecordDecl` but currently isn't the
> > > same object within `C`, but can be, if `B` declares `A` as a virtual
> > > base).
> > So, even it is some kind of anti-pattern as a warning arrive immediately,
> > now I allow `B` to `C` downcasts. Could you explain me more about that
> > virtual idea, please? Based on this possible use-case in my mind two
> > classes are on the same level as every of their bases/vbases are equals.
> > Could you explain me more about that virtual idea, please?
>
> In the following variation:
> ```
> struct A {};
> struct B : virtual A {};
> struct C : A, B {};
> ```
> we have `C` contain only one instance of `A`: the layout of `B` within `C`
> would merely refer to the instance of `A` within `C` instead of making its
> own copy. In particular, the constructor of `B` within the constructor of `C`
> would not initialize `C`'s instance of `A` because the constructor of `C`
> will invoke the constructor of `A` directly (cf. D61816).
That is a cool example, thanks you!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:174
+
+constexpr llvm::StringLiteral Vowels = "aeiou";
+
----------------
NoQ wrote:
> 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".
That is a cool idea, I will adjust `StringExtras.h` after that patch, for now
please let us use that simple heuristic.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67079/new/
https://reviews.llvm.org/D67079
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
