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

martong added a comment. In D55134#3224339 , @ychen wrote: > FWIW, ASTUnit seems to save an effective triple while the current AST uses > the nominal triple. (for example, `armv7a-xx-xx-eabihf` vs > `armv7a-xx-xx-eabi`). I'm not sure if there is a good s

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

ychen added a comment. Herald added a reviewer: Szelethus. Herald added a subscriber: steakhal. FWIW, ASTUnit seems to save an effective triple while the current AST uses the nominal triple. (for example, `armv7a-xx-xx-eabihf` vs `armv7a-xx-xx-eabi`). I'm not sure if there is a good solution oth

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL348610: [CTU] Add triple/lang mismatch handling (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55134?vs=176687&id=17722

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

xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LG! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 ___ cfe-commi

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

martong added a comment. > We should probably prefer %clang_analyze_cc1 to %clang_cc1 -analyze here too. Ok, changed that. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 ___

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

martong updated this revision to Diff 176687. martong added a comment. - Use clang_analyze_cc1 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 Files: include/clang/Basic/DiagnosticCrossTUKinds.td include/clang/Basic/D

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

martong updated this revision to Diff 176686. martong marked an inline comment as done. martong added a comment. - Add a case to emitCrossTUDiagnostics Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 Files: include/clan

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

martong marked 2 inline comments as done. martong added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212 +// diagnostics. +Context.getDiagnostics().Report(diag::err_ctu_incompat_triple) +<< Unit->getMainFileName() << TripleTo.str() << TripleFr

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

Szelethus added a comment. We should probably prefer `%clang_analyze_cc1` to `%clang_cc1 -analyze` here too. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 ___ cfe-commits m

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

xazax.hun added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:212 +// diagnostics. +Context.getDiagnostics().Report(diag::err_ctu_incompat_triple) +<< Unit->getMainFileName() << TripleTo.str() << TripleFrom.str(); martong wrote

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

martong updated this revision to Diff 176672. martong added a comment. - Forgot to rename err_ctu_incompat_triple -> warn_ctu_incompat_triple Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 Files: include/clang/Basic/Di

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

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 sur

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

martong updated this revision to Diff 176671. martong marked 4 inline comments as done. martong added a comment. - Diagnose a warning (which may be upgradable to an error) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 F

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

xazax.hun requested changes to this revision. xazax.hun added inline comments. This revision now requires changes to proceed. Comment at: include/clang/Basic/DiagnosticCrossTUKinds.td:19 + +def err_ctu_incompat_triple : Error< + "imported AST from '%0' had been generated for a d

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

martong updated this revision to Diff 176584. martong added a comment. - Break long RUN lines Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 Files: include/clang/Basic/DiagnosticCrossTUKinds.td include/clang/CrossTU/

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

a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 ___

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

martong added a comment. > Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be > an issue there? I really don't know. We never tried it, our focus is on C and C++ for now. Unfortunately, there is nobody with ObjC/C++ knowledge and (more importantly) with interest to t

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

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 w

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

martong updated this revision to Diff 176417. martong marked 13 inline comments as done. martong added a comment. - Address review comments Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 Files: include/clang/Basic/Diag

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

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 it

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

Szelethus added a comment. Hah. Do we support CTU for other languages, like ObjC and ObjC++? Can this be an issue there? Comment at: lib/CrossTU/CrossTranslationUnit.cpp:59 +} +} + `// end of namespace llvm` Comment at: lib/CrossTU/CrossTran

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

martong updated this revision to Diff 176172. martong added a comment. - Add lit tests Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55134/new/ https://reviews.llvm.org/D55134 Files: include/clang/Basic/DiagnosticCrossTUKinds.td include/clang/CrossTU/CrossTr

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

martong created this revision. martong added reviewers: xazax.hun, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. We introduce a strict policy for C++ CTU. It can work across TUs only if the C++ dialects are the same. We neither allow C vs C++ CTU. We do