[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. juliehockett marked 4 inline comments as done. Closed by commit rCTE323011: [clang-tidy] Adding Fuchsia checker for multiple inheritance (aut

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a few small nits. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:85 +void MultipleInheritanceCheck::registerMatchers(MatchFinder *Finder) { + // Match declarations which have base

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 130427. juliehockett added a comment. Rebasing from trunk https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tidy/fuchsia/Multi

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

aaron.ballman 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(); rsmith

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

rsmith 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(); aaron.ballman

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 129498. juliehockett marked 8 inline comments as done. juliehockett added a comment. 1. Updating check and tests to address virtual inheritance 2. Rebasing from trunk https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-ti

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

aaron.ballman added inline comments. Comment at: test/clang-tidy/fuchsia-multiple-inheritance.cpp:48 +}; + +// Inherits from multiple concrete classes. juliehockett wrote: > aaron.ballman wrote: > > The virtual base test cases I was thinking of were: > > ``` > >

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett added inline comments. Comment at: test/clang-tidy/fuchsia-multiple-inheritance.cpp:48 +}; + +// Inherits from multiple concrete classes. aaron.ballman wrote: > The virtual base test cases I was thinking of were: > ``` > struct Base { virtual void fo

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 125449. juliehockett marked an inline comment as done. https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tidy/fuchsia/MultipleIn

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

aaron.ballman 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(); julieho

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

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(); aaron.ba

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 125215. juliehockett marked 7 inline comments as done. juliehockett added a comment. Updating tests https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInher

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35 + StringRef Name = Node->getIdentifier()->getName(); + auto Pair = InterfaceMap.find(Name); + if (Pair == InterfaceMap.end()) Eugene.Zelenko wrote: > aaron.ball

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

Eugene.Zelenko added inline comments. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35 + StringRef Name = Node->getIdentifier()->getName(); + auto Pair = InterfaceMap.find(Name); + if (Pair == InterfaceMap.end()) aaron.ballman wrote: > Don't use

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

aaron.ballman added inline comments. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35 + StringRef Name = Node->getIdentifier()->getName(); + auto Pair = InterfaceMap.find(Name); + if (Pair == InterfaceMap.end()) Don't use `auto` as the type is no

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 125000. juliehockett marked 4 inline comments as done. https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tidy/fuchsia/MultipleIn

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

alexfh added inline comments. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:35-36 + StringRef Name = Node->getIdentifier()->getName(); + if (InterfaceMap.count(Name)) { +isInterface = InterfaceMap.lookup(Name); +return true; One lookup is

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 124949. juliehockett added a comment. Updated warning wording to more accurately reflect guidelines https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInher

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

rsmith added inline comments. Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46 + +See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md juliehockett wrote: > alexfh wrote: > > This is not abo

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett added inline comments. Comment at: docs/clang-tidy/checks/fuchsia-multiple-inheritance.rst:46 + +See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md alexfh wrote: > This is not about the check, rath

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 124853. juliehockett added a comment. Rebasing for updated Release Notes -- so much nicer :) https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceC

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

Eugene.Zelenko added a comment. Please rebase from trunk. I just enforced some order in chaos of Release Notes :-) https://reviews.llvm.org/D40580 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 124772. juliehockett marked 11 inline comments as done. juliehockett added a comment. Moved AST matcher to ASTMatchers.h (see D40611 ), addressing comments. https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/C

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

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.Ze

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:22 +AST_MATCHER(CXXRecordDecl, hasDefinition) { + if (!Node.hasDefinition()) +return false; ---

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 124675. juliehockett added a comment. Fixing typo https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tidy/fuchsia/MultipleInher

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 124669. juliehockett marked 4 inline comments as done. https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tidy/fuchsia/MultipleIn

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

Eugene.Zelenko added inline comments. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:76 + for (const auto &I : Node->bases()) { +const RecordType *Ty = I.getType()->getAs(); +assert(Ty && "RecordType of base class is unknown"); Could be cons

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett updated this revision to Diff 124662. juliehockett marked 6 inline comments as done. https://reviews.llvm.org/D40580 Files: clang-tidy/fuchsia/CMakeLists.txt clang-tidy/fuchsia/FuchsiaTidyModule.cpp clang-tidy/fuchsia/MultipleInheritanceCheck.cpp clang-tidy/fuchsia/MultipleIn

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

Eugene.Zelenko added inline comments. Comment at: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp:21 + + AST_MATCHER(CXXRecordDecl, hasDefinition) { +if (!Node.hasDefinition()) Please reduce indentation level. Comment at: clang-tidy/fuchsi

[PATCH] D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance

juliehockett created this revision. juliehockett added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Adds a check to the Fuchsia module to warn when a class inheritsfrom multiple classes that are not pure virtual. See https://fuchsia.googlesource.com/zircon/+/master/