juliehockett added inline comments.
================
Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:60
+ // To be an interface, all base classes must be interfaces as well.
+ for (const auto &I : Node->bases()) {
+ const auto *Ty = I.getType()->getAs<RecordType>();
----------------
aaron.ballman wrote:
> What about virtual bases (`Node->vbases()`)? This would also be worth some
> test cases.
Added test cases for virtual, but aren't virtual bases also included in
`bases()`?
================
Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:77
+ // Match declarations which have definitions.
+ Finder->addMatcher(cxxRecordDecl(hasDefinition()).bind("decl"), this);
+}
----------------
aaron.ballman wrote:
> It might be nice to not bother matching class definitions that have no bases
> or virtual bases rather than matching every class definition. However, this
> could be done in a follow-up patch (it likely requires adding an AST matcher).
Good point -- will do.
================
Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.h:34-36
+ bool getInterfaceStatus(const CXXRecordDecl *Node, bool &isInterface);
+ bool isCurrentClassInterface(const CXXRecordDecl *Node);
+ bool isInterface(const CXXRecordDecl *Node);
----------------
aaron.ballman wrote:
> I believe all these methods can be marked `const`.
getInterfaceStatus and isInterface can't be -- they update the map. The other
ones yes though!
https://reviews.llvm.org/D40580
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
