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

Tue Dec 4 10:31:29 PST 2018

martong added inline comments.

================
Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19
+
+def err_ctu_incompat_triple : Error<
+  "imported AST from '%0' had been generated for a different target, current: %1, imported: %2">;
----------------
xazax.hun wrote:
> I am not sure if we want this to be an error. The analysis could continue without any problems if we do not actually merge the two ASTs. So maybe having a warning only is better. My concern is that once we have this error, there would be no way to analyze mixed language (C/C++) projects cleanly using CTU.
Okay, I agree, I changed it to be a warning by default.

================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212
+    // diagnostics.
+    Context.getDiagnostics().Report(diag::err_ctu_incompat_triple)
+        << Unit->getMainFileName() << TripleTo.str() << TripleFrom.str();
----------------
xazax.hun wrote:
> I think we should not emit an error here. It should be up to the caller (the user of the library) to decide if she wants to handle this as an error, warnings, or just suppress these kinds of problems. I would rather extend `emitCrossTUDiagnostics` as a shorthand for the user if emitting an error is the desired behavior.
 I would prefer to exploit the capabilities of the `DiagEngine`, instead of extending the interface of `CrossTranslationUnitContext`.
By using a `DiagGroup` we can upgrade the warning to be an error, so I just changed it to be like that.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55134/new/

https://reviews.llvm.org/D55134