aaron.ballman added inline comments.
================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:58 + // Interfaces should have exclusively pure methods. + for (auto method : Node->methods()) { + if (method->isUserProvided() && !method->isPure()) { ---------------- Eugene.Zelenko wrote: > const auto? Should be `const auto *`; also `method` does not meet our naming convention rules. Actually, the whole thing can be replaced by `return llvm::none_of(Node->methods(), [](const CXXMethodDecl *M) { return M->isUserProvided() && !M->isPure(); });` ================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:21 + +AST_MATCHER(CXXRecordDecl, hasDefinition) { + if (!Node.hasDefinition()) ---------------- This is another example of an AST matcher that should simply be made public in ASTMatchers.h (as a separate commit). Feel free to add me as a reviewer to that patch (it should be pretty trivial). ================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:29-30 +// previously. +void MultipleInheritanceCheck::addNodeToInterfaceMap(const CXXRecordDecl *Node, + bool isInterface) { + StringRef Name = Node->getIdentifier()->getName(); ---------------- Formatting is off here -- be sure to run the patch through clang-format. ================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:40 +bool MultipleInheritanceCheck::getInterfaceStatus(const CXXRecordDecl *Node, + bool *isInterface) { + StringRef Name = Node->getIdentifier()->getName(); ---------------- Rather than passing a pointer, why not pass a reference? ================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:49 + +// TODO(smklein): Make an exception for 'Dispatcher' -- consider it an +// interface, even though it isn't. ---------------- We don't put names into our TODO or FIXME comments. ================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:54 + // Interfaces should have no fields. + if (!Node->field_empty()) { + return false; ---------------- Can elide braces, here and elsewhere. ================ Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:102 + // concrete classes + int NumConcrete = 0; + for (const auto &I : D->bases()) { ---------------- Might as well make this `unsigned` rather than `int`. https://reviews.llvm.org/D40580 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits