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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
