[PATCH] D55134: [CTU] Add triple/lang mismatch handling

[Gabor Marton via Phabricator via cfe-commits]

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