martong added a comment. Thanks for the review! I have updated the patch according to your comments.
================ Comment at: lib/CrossTU/CrossTranslationUnit.cpp:31 +namespace llvm { +// Same as Triple's equality operator, but we check a field only if that is ---------------- a_sidorin wrote: > Why don't we place it into the anon namespace just below? Ok, I moved it into the unnamed namespace below. ================ Comment at: lib/CrossTU/CrossTranslationUnit.cpp:35 +bool hasEqualKnownFields(const Triple &Lhs, const Triple &Rhs) { + return ((Lhs.getArch() != Triple::UnknownArch && + Rhs.getArch() != Triple::UnknownArch) ---------------- a_sidorin wrote: > This has to be split, probably with early returns. Example: > ```if (Lhs.getArch() != Triple::UnknownArch && Rhs.getArch() != > Triple::UnknownArch && > Lhs.getArch() != Rhs.getArch() > return false; > ...``` Ok, I split that up with early returns. ================ Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59 +} +} + ---------------- a_sidorin wrote: > Szelethus wrote: > > `// end of namespace llvm` > `// end namespace llvm` is much more common. Moved to the unnamed namespace. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits