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

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. > Exactly. And this seems to be an important feature in ASTImporter, because > this makes it possible that we can chain into the redecl chain a newly > imported AST node to existing and structurally equivalent > nodes. (The > existing nodes are found by the lookup an

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

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. In D53708#1332922 , @riccibruno wrote: > In D53708#1332868 , @martong wrote: > > > > This is a perhaps a naive comment, but why is localUncachedLookup used > > > instead of noload_lookup ?

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

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. In D53708#1332868 , @martong wrote: > > This is a perhaps a naive comment, but why is localUncachedLookup used > > instead of noload_lookup ? There is a fixme dating from 2013 stating > > that noload_lookup should be used inst

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

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. > This is a perhaps a naive comment, but why is localUncachedLookup used instead of noload_lookup ? There is a fixme dating from 2013 stating that noload_lookup should be used instead. This is not a naive question. I was wondering myself on the very same thing when I st

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

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL349351: [ASTImporter] Add importer specific lookup (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

2018-12-17 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. This is a perhaps a naive comment, but why is `localUncachedLookup` used instead of `noload_lookup` ? There is a fixme dating from 2013 stating that `noload_lookup` should be used instead. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

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

2018-12-17 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 178452. martong added a comment. - Rebase to master (trunk) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53708/new/ https://reviews.llvm.org/D53708 Files: include/clang/AST/ASTImporter.h include/clang/AST/ASTImporterLoo

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

2018-12-01 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin accepted this revision. a_sidorin added a comment. This revision is now accepted and ready to land. Hi Gabor, I think the code looks good. Thank you! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53708/new/ https://reviews.llvm.org/D53708 ___

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

2018-11-30 Thread Gabor Marton via Phabricator via cfe-commits
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] D53708: [ASTImporter] Add importer specific lookup

2018-11-30 Thread Gabor Marton via Phabricator via cfe-commits
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

2018-11-30 Thread Gabor Marton via Phabricator via cfe-commits
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

2018-11-28 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment. Hi Gabor, Here are some new 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 contexts. --

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

2018-11-27 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 175495. martong marked 5 inline comments as done. martong added a comment. - Address Alexei's comments Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53708/new/ https://reviews.llvm.org/D53708 Files: include/clang/AST/ASTIm

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

2018-11-27 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 18 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:7605 + +ASTImporter::ASTImporter(ASTImporterLookupTable *LookupTable, + ASTContext &ToContext, FileManager &ToFileManager, a_sido

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

2018-11-25 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a subscriber: jingham. a_sidorin added a comment. Hi Gabor, I think it is a good patch with a nice test set. There are some mine comments inline. I'd also like to have LLDB guys opinion on it. Comment at: lib/AST/ASTImporter.cpp:7605 + +ASTImporter::ASTImporte

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

2018-11-22 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Constructs like `struct A { struct Foo *p; };` are very common in C projects. Since `Foo` as a forward decl cannot be found by normal lookup we need this patch in order to properly CTU analyze even a simple C project like `tmux`. Repository: rC Clang https://reviews

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

2018-11-22 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411. Gentle Ping. Repository: rC Clang https://reviews.llvm.org/D53708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

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

2018-10-25 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs, mgorny. Herald added a reviewer: a.sidorin. There are certain cases when normal C/C++ lookup (localUncachedLookup) does not find AST nodes. E.g.: Example 1: t