martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Shafik, thanks for addressing the comments. This looks good to me now!
================
Comment at: lib/AST/ASTImporter.cpp:3042
+ if (FoundByLookup) {
+ if (auto *MD = dyn_cast<CXXMethodDecl>(FoundByLookup)) {
----------------
shafik wrote:
> martong wrote:
> > It is not trivial why we add this block to the code, so I think a comment
> > would be really appreciated here.
> > I was thinking about something like this:
> > ```
> > + // We do not allow more than one in-class prototype of a function. This
> > is
> > + // because AST clients like VTableBuilder asserts on this. VTableBuilder
> > + // assumes that only one function can override a function. Building a
> > redecl
> > + // chain would result there are more than one function which override the
> > + // base function (even if they are part of the same redecl chain inside
> > the
> > + // derived class.)
> > ```
> Just for clarification, from a C++ perspective, I would say "in-class
> declaration" rather than "in-class prototype" and therefore `VTableBuilder`
> assumes only one in class declaration and building a redecal chain would
> result in more than one in-class declaration being present.
>
> Does that makes sense?
Yes, absolutely. Please change the comment as you suggest.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56936/new/
https://reviews.llvm.org/D56936
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
