[PATCH] D53751: [ASTImporter] Added Import functions for transition to new API.

martong added a comment. > I didn't actually see this comment get addressed other than to say it won't > be a problem in practice, which I'm not certain I agree with. Was there a > reason why this got commit before finding out if the reviewer with the > concern agrees with your rationale? FWIW,

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong updated this revision to Diff 175647. martong added a comment. - Use ExtInfo in structural equivalency Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/

[PATCH] D53699: [ASTImporter] Fix inequality of functions with different attributes

martong added a comment. Hi @a_sidorin , I have updated the patch as you suggested, to check the equivalence based on `ASTContext::mergeFunctionTypes()`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53699/new/ https://reviews.llvm.org/D53699 ___

[PATCH] D53693: [ASTImporter] Typedef import brings in the complete type

martong marked an inline comment as done. martong added inline comments. Comment at: cfe/trunk/unittests/AST/ASTImporterTest.cpp:3780 + typedefNameDecl(hasName("U"))); + ASSERT_TRUE(ToD->getUnderlyingType()->isIncompleteType()); + shafik wrote: > As far as

[PATCH] D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.

martong added a comment. > I reverted this change because it breaks a bunch of lldb tests (on MacOS, and > probably on other platforms too). To be clear, part of the reason I'm > reacting strongly here is that this is not the first patch this has come up > on. I'm worried about the broader tren

[PATCH] D53708: [ASTImporter] Add importer specific lookup

martong marked 8 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:7658 +ASTImporter::FoundDeclsTy +ASTImporter::FindDeclsInToCtx(DeclContext *DC, DeclarationName Name) { + // We search in the redecl context because of transparent contex

[PATCH] D53708: [ASTImporter] Add importer specific lookup

martong updated this revision to Diff 176100. martong marked 3 inline comments as done. martong added a comment. - Address Alexei's comments - Rename FindDeclsInToCtx to findDeclsInToCtx - Remove superfluous spaces from stringliterals - Remove unused header - Remove empty test - Return nullptr and

[PATCH] D53708: [ASTImporter] Add importer specific lookup

martong added a comment. Hey Alexei, Thank you again for your comments. I changed to code accordingly. Also, I recognized that we should use a `SmallSetVector` instead of a `SmallPtrSet`, because the latter results unpredictable iteration (pointers may have different values with each run). Tho

[PATCH] D55129: [CTU] Eliminate race condition in CTU lit tests

martong created this revision. martong added reviewers: xazax.hun, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. We plan to introduce additional CTU related lit test. Since lit may run the tests in parallel, it is not safe to use the same directory (%T)

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong created this revision. martong added reviewers: xazax.hun, balazske, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Adding some more CTU list tests. E.g. to check if a construct is unsupported. We also slightly modify the handling of the return va

[PATCH] D55132: [CTU] Add asserts to protect invariants

martong created this revision. martong added reviewers: xazax.hun, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Repository: rC Clang https://reviews.llvm.org/D55132 Files: lib/CrossTU/CrossTranslationUnit.cpp Index: lib/CrossTU/CrossTranslationU

[PATCH] D55133: [CTU] Add statistics

martong created this revision. martong added reviewers: xazax.hun, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Repository: rC Clang https://reviews.llvm.org/D55133 Files: lib/CrossTU/CrossTranslationUnit.cpp Index: lib/CrossTU/CrossTranslationU

[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

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong created this revision. martong added reviewers: xazax.hun, Szelethus, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity. Herald added a reviewer: george.karpenkov. With a new switch

[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] D44100: [ASTImporter] Reorder fields after structure import is finished

martong added a comment. Alexei, I had a chance to have a quick look into this on a borrowed Mac Book. Seems like the assertion which was related to the revert is not existent anymore, but a new assertion came in. Next week I'll have time to debug that and I'll come back to you with what I fin

[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

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 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] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong updated this revision to Diff 176581. martong marked 4 inline comments as done. martong added a comment. - Rename AnalyzerDisplayCtuProgress to Opts.AnalyzerDisplayCTUProgress - Change the CTU progress message Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong added a comment. > Also, almost everywhere CTU is capitalized, so I guess it should be in the > field name too. Ok, I renamed it to have CTU all capitalized. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:239 + if (DisplayCTUProgress) { +llvm::errs() <<

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong updated this revision to Diff 176582. martong added a comment. - Change the CTU progress message in the test too Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 Files: include/clang/CrossTU/CrossTranslationUnit.

[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] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong added inline comments. Comment at: test/Analysis/ctu-main.cpp:6 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ct

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong updated this revision to Diff 176590. martong marked an inline comment as done. martong added a comment. - Break long RUN line Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 Files: include/clang/CrossTU/CrossTr

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong updated this revision to Diff 176619. martong marked 8 inline comments as done. martong added a comment. - Break long RUN lines - Clang format the test files - Use consistent naming style - Remove braces Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55131/

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong added inline comments. Comment at: test/Analysis/Inputs/ctu-other.c:6 + int b; +} foobar; + a_sidorin wrote: > Please use a consistent naming style across the file. There are names > starting with capital, having underscores and written like this. Ok, I

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong added a comment. > Also, maybe it'd be worth making a CTU directory under test/Analysis for CTU > related test files. It is a good point, but I'd do that in the future once we have even more ctu related test files. Perhaps together with a new check-clang-analysis-ctu build target. Re

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong updated this revision to Diff 176653. martong added a comment. - Use clang_analyze_cc1 - Change to be an analyzer config option Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 Files: include/clang/CrossTU/CrossT

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong added a comment. > should be an -analyzer-config option. Ok, just changed it to be. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 ___ cfe-commits mailing list cfe-c

[PATCH] D55133: [CTU] Add statistics

martong updated this revision to Diff 176654. martong added a comment. - Remove NumNoUnit Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55133/new/ https://reviews.llvm.org/D55133 Files: lib/CrossTU/CrossTranslationUnit.cpp Index: lib/CrossTU/CrossTranslation

[PATCH] D55133: [CTU] Add statistics

martong added a comment. > Sorry, but I don't understand the meaning of some options. Could you please > explain what are NumNoUnit and NumNotInOtherTU and what is the difference > between them? Your point is absolutely true. They are the same, I think at some point some time ago they were dif

[PATCH] D55133: [CTU] Add statistics

martong updated this revision to Diff 176655. martong marked an inline comment as done. martong added a comment. - Remove braces Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55133/new/ https://reviews.llvm.org/D55133 Files: lib/CrossTU/CrossTranslationUnit.cp

[PATCH] D55133: [CTU] Add statistics

martong added a comment. >> Sorry, but I don't understand the meaning of some options. Could you please >> explain what are NumNoUnit and NumNotInOtherTU and what is the difference >> between them? > Your point is absolutely true. They are the same, I think at some point some > time ago they

[PATCH] D55280: [CTU] Remove redundant error check

martong created this revision. martong added reviewers: xazax.hun, a_sidorin, Szelethus, balazske. Herald added subscribers: cfe-commits, gamesh411, dkrupp, rnkovacs. In loadExternalAST we normally return with either an error or with a valid ASTUnit pointer which should not be a nullptr. However,

[PATCH] D55129: [CTU] Eliminate race condition in CTU lit tests

martong updated this revision to Diff 176660. martong added a comment. - Use rm, mkdir and break long RUN lines Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55129/new/ https://reviews.llvm.org/D55129 Files: test/Analysis/ctu-main.cpp Index: test/Analysis/ct

[PATCH] D53655: [ASTImporter] Fix redecl chain of classes and class templates

martong added a comment. @rsmith ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53655/new/ https://reviews.llvm.org/D53655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[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

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 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

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 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 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] D55280: [CTU] Remove redundant error check

martong updated this revision to Diff 176779. martong added a comment. - Return an error from loadExternalAST in case of nullptr Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55280/new/ https://reviews.llvm.org/D55280 Files: include/clang/CrossTU/CrossTranslat

[PATCH] D55280: [CTU] Remove redundant error check

martong added a comment. @Szelethus @balazske Thanks for your review! The meantime I have discussed with @xazax.hun that actually the called `ASTUnit::LoadFromASTFile` function inside `loadExternalAST` may return with a nullptr. So, the best is to handle that inside `loadExternalAST`. Reposit

[PATCH] D55280: [CTU] Make loadExternalAST return with non nullptr on success

martong marked an inline comment as done. martong added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:147 llvm::Expected CrossTranslationUnitContext::getCrossTUDefinition(const FunctionDecl *FD, balazske wrote: > Szelethus wrote: > > Would

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong marked an inline comment as done. martong added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:247 llvm::Expected CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) { ASTImporter &Importer = getOrCreateASTImporter(FD->getASTConte

[PATCH] D55280: [CTU] Make loadExternalAST return with non nullptr on success

martong added a comment. Hi Aleksey, The first version was indeed not correct. But, I think we handle that case with the latest update (https://reviews.llvm.org/differential/diff/176779/). In `loadExternalAST()` we have this right before the return: if (!Unit) return llvm::make_error(

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong added a comment. Hey Alexey, Here is what I found so far: The new assertion is this: Assertion failed: (ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD)), function ImportDeclContext ToRD looks like this in code: typedef struct __sFILE { // several fields here ... }

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong marked 2 inline comments as done. martong added inline comments. Comment at: test/Analysis/ctu-main.c:4 +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c +// RUN: cp %S/Inputs/externalFnMap2.txt %t/ctudir2/exter

[PATCH] D55131: [CTU] Add more lit tests and better error handling

martong updated this revision to Diff 177170. martong marked an inline comment as done. martong added a comment. - Rename externalFnMap files Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55131/new/ https://reviews.llvm.org/D55131 Files: lib/CrossTU/CrossTrans

[PATCH] D55133: [CTU] Add statistics

This revision was automatically updated to reflect the committed changes. Closed by commit rL348584: [CTU] Add statistics (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55133?vs=176655&id=177173#toc Repository:

[PATCH] D55133: [CTU] Add statistics

martong marked 2 inline comments as done. martong added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:171 loadExternalAST(LookupFnName, CrossTUDir, IndexName); - if (!ASTUnitOrError) + if (!ASTUnitOrError) { return ASTUnitOrError.takeError(); ---

[PATCH] D55132: [CTU] Add asserts to protect invariants

This revision was automatically updated to reflect the committed changes. Closed by commit rL348586: [CTU] Add asserts to protect invariants (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55132?vs=176151&id=17717

[PATCH] D55129: [CTU] Eliminate race condition in CTU lit tests

This revision was automatically updated to reflect the committed changes. Closed by commit rL348587: [CTU] Eliminate race condition in CTU lit tests (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong marked an inline comment as done. martong added a comment. > While Static Analyzer is the only client of CTU library at the moment, we > might have more in the future. I would not use the phrase ANALYZE in the log > message. Once this is resolved the rest looks good. Ok, I removed the "

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

martong updated this revision to Diff 177204. martong added a comment. - Remove 'ANALYZE ' prefix from the log message Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55135/new/ https://reviews.llvm.org/D55135 Files: include/clang/CrossTU/CrossTranslationUnit.h

[PATCH] D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch

This revision was automatically updated to reflect the committed changes. Closed by commit rL348594: [CTU] Add DisplayCTUProgress analyzer switch (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55135?vs=177204&id=

[PATCH] D55131: [CTU] Add more lit tests and better error handling

This revision was automatically updated to reflect the committed changes. Closed by commit rL348605: [CTU] Add more lit tests and better error handling (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D55131?vs=1771

[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] D53655: [ASTImporter] Fix redecl chain of classes and class templates

martong added a comment. @rsmith Please raise any objections until Dec 14 (or if this deadline is too strict). I'd like to commit this next week latest so it can get in still this year. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53655/new/ https://reviews.ll

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong added a comment. @a_sidorin The below diff on top of your patch successfully handles the failure with the `TestCModules.py` LLDB testcase: diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index 05fec7f943..e6fb590025 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/A

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong added a comment. There is another failure on macOS, which is not there on Linux. This is present with the 174545 patch id (even before applying my fix for TestCModules). $ ninja check-clang-astmerge Testing Time: 0.63s Failing Tests (4)

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong added a comment. Alexey, good news! I have finished the testing of the above patch applied on top of 174545. Neither is a regression on Linux nor on macOS. (ASTTests, check-clang-astmerge, check-clang-import, check-clang-analysis, check-lldb). Repository: rC Clang CHANGES SINCE LAST

[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:1344 + + EXPECT_EQ(FromChooseExpr->isConditionTrue(), ToChooseExpr->isConditionTrue()); +} To compensate the skipping of the template test, perhaps we should have another expe

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. Redecl chains of function template specializations are not handled well curre

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. Redecl chains of class template specializations are not handled w

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

martong updated this revision to Diff 188379. martong marked 4 inline comments as done. martong added a comment. - getDefinition -> getTemplateDefinition - Remove comments for braces, added FIXME Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58494/new/ https://re

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4966 // it has any definition in the redecl chain. -static ClassTemplateDecl *getDefinition(ClassTemplateDecl *D) { - CXXRecordDecl *ToTemplatedDef = D->getTemplatedDecl()->getDefinition(); +template static aut

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

martong marked 2 inline comments as done. martong added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:255 + // in the other unit it has none. + if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 || + LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 || ---

[PATCH] D57590: [ASTImporter] Improve import of FileID.

martong added a comment. In D57590#1411055 , @efriedma wrote: > I don't know anything about this particular patch, but you aren't allowed to > set deadlines like that; the patch review process requires that the patch is > actually reviewed. If you aren'

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

martong accepted this revision. martong added a reviewer: balazske. martong added a subscriber: balazske. martong added a comment. Shafik, this looks good to me, once teemperor's comments are addressed. Note, I added @balazske as a reviewer, he recently worked with importing the FileIDs, he may h

[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rC355096: [CTU] Do not allow different CPP dialects in CTU (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D57906?vs=18592

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

martong added a comment. @shafik, Could you please take a look? I have run the LLDB tests on our macOS and I could not discover any regression. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58502/new/ https://reviews.llvm.org/D58502 ___

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

martong added a comment. @shafik, Could you please take a look? I have run the LLDB tests on our macOS and I could not discover any regression. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58494/new/ https://reviews.llvm.org/D58494 __

[PATCH] D58830: [ASTImporter] Import member expr with explicit template args

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. Member expressions with explicit template arguments were not imported correct

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

martong marked 8 inline comments as done. martong added a comment. Alexei, thanks for the review! Comment at: lib/AST/ASTImporter.cpp:3002 + // Check if we have found an existing definition. Returns with that + // definition if yes, otherwise returns null.

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

martong updated this revision to Diff 189130. martong marked 4 inline comments as done. martong added a comment. - Add FindAndMapDefinition as member fun - Refactor CheckPreviousDecl Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58668/new/ https://reviews.llvm.or

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

martong updated this revision to Diff 189131. martong marked 2 inline comments as done. martong added a comment. - Fix some comments Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58673/new/ https://reviews.llvm.org/D58673 Files: lib/AST/ASTImporter.cpp unitt

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

martong added a comment. Alexei, thanks for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58673/new/ https://reviews.llvm.org/D58673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.l

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

This revision was automatically updated to reflect the committed changes. Closed by commit rL355390: [ASTImporter] Fix redecl failures of Class and ClassTemplate (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHA

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

martong updated this revision to Diff 189665. martong marked 6 inline comments as done. martong added a comment. - Add asserts Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58494/new/ https://reviews.llvm.org/D58494 Files: lib/AST/ASTImporter.cpp unittests/A

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4967 +template static auto getTemplateDefinition(T *D) -> T * { + auto *ToTemplatedDef = D->getTemplatedDecl()->getDefinition(); if (!ToTemplatedDef) shafik wrote: > shafik wrote: > > Can we

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

This revision was automatically updated to reflect the committed changes. Closed by commit rC355593: [ASTImporter] Handle redecl chain of FunctionTemplateDecls (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D58494?vs=189665&id=189694#toc Repository: r

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

martong marked 2 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:5147 } +} else { // ODR violation. + // FIXME HandleNameConflict shafik wrote: > ODR violations are ill-formed no diagnostic required. So

[PATCH] D58830: [ASTImporter] Import member expr with explicit template args

This revision was automatically updated to reflect the committed changes. Closed by commit rL355596: [ASTImporter] Import member expr with explicit template args (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHA

[PATCH] D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

martong added a comment. Herald added a reviewer: martong. Herald added a project: clang. Ping. This patch is the next in the series of patches to finish the proper error handling (i.e. using Error and Expected). Please see the "Stack" column above. Repository: rC Clang CHANGES SINCE LAST A

[PATCH] D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

martong added a comment. Herald added a subscriber: rnkovacs. > This patch is the next in the series of patches to finish the proper error > handling (i.e. using Error and Expected). Please see the "Stack" column > above. @balazske Or is it superseded by this one? https://reviews.llvm.org/D5381

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong updated this revision to Diff 189830. martong edited the summary of this revision. martong added a comment. Herald added a project: clang. Rebase to master. There was a conflict in the tests, in ASTImporterTest.cpp. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

martong commandeered this revision. martong edited reviewers, added: a_sidorin; removed: martong. martong added a comment. This revision now requires review to proceed. Herald added subscribers: gamesh411, dkrupp. @a_sidorin Aleksei, If you don't mind, I am going to investigate this further with

[PATCH] D58897: [ASTImporter] Make ODR error handling configurable

martong added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58897/new/ https://reviews.llvm.org/D58897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

martong added a comment. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58668/new/ https://reviews.llvm.org/D58668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D55358: [ASTImporter] Fix import of NestedNameSpecifierLoc.

martong added a reviewer: a_sidorin. martong added a comment. Herald added a reviewer: martong. Herald added a subscriber: jdoerfert. Herald added a project: clang. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55358/new/ https://reviews.llvm.org/D55358 _

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

martong added a comment. @shafik Ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58668/new/ https://reviews.llvm.org/D58668 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

martong marked an inline comment as done. martong added a comment. @shafik Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58673/new/ https://reviews.llvm.org/D58673 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

martong added a comment. > We can't reliable import templates with the ASTImporter, so actually > reconstructing the template in the debug info AST and then importing it > doesn't work. Could you please elaborate how the import of templates fails in ASTImporter? Is it because the AST you build

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

This revision was automatically updated to reflect the committed changes. Closed by commit rL356452: [ASTImporter] Fix redecl failures of ClassTemplateSpec (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

martong updated this revision to Diff 191286. martong added a comment. - Rebase to master Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58668/new/ https://reviews.llvm.org/D58668 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unitte

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

This revision was automatically updated to reflect the committed changes. Closed by commit rL356455: [ASTImporter] Fix redecl failures of FunctionTemplateSpec (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGE

<    1   2   3   4   5   6   7   8   9   10   >