a_sidorin added a comment.
Hi Gabor,
Please find my comments inline.
> Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be
> an issue there?
That's a good question. In Samsung, CTU hasn't been tested on ObjC code.
Upstream CTU supports only FunctionDecls as well so its ObjC status is
questionable..
================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:31
+namespace llvm {
+// Same as Triple's equality operator, but we check a field only if that is
----------------
Why don't we place it into the anon namespace just 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)
----------------
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;
...```
================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:47
+ : true) &&
+ ((Lhs.getOS() != Triple::UnknownOS && Rhs.getOS() !=
Triple::UnknownOS)
+ ? Lhs.getOS() == Rhs.getOS()
----------------
We can use `Triple::isOSUnknown()` instead.
================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59
+}
+}
+
----------------
Szelethus wrote:
> `// end of namespace llvm`
`// end namespace llvm` is much more common.
================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:203
+ const auto& TripleTo = Context.getTargetInfo().getTriple();
+ const auto& TripleFrom = Unit->getASTContext().getTargetInfo().getTriple();
----------------
Reference char should stick to the variable, not to the type.
================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:211
+ // TODO pass the SourceLocation of the CallExpression for more precise
+ // diagnostics
+ Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
----------------
Comments should end with a dot. Same below.
================
Comment at: test/Analysis/ctu-unknown-parts-in-triples.cpp:8
+// RUN: cp %S/Inputs/externalFnMap.txt %t/ctudir/externalFnMap.txt
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -analyze
-analyzer-checker=core,debug.ExprInspection -analyzer-config
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%t/ctudir
-verify %s
+
----------------
Two spaces after `ExprInspection`.
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
