[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

martong added inline comments. Comment at: lib/StaticAnalyzer/Core/IssueHash.cpp:39 + // primary template. + if (const FunctionDecl *InstantiatedFrom = + Target->getInstantiatedFromMemberFunction()) Could we use here FunctionDecl::getPrimaryTemplate()

[PATCH] D38728: [analyzer] Use the signature of the primary template for issue hash calculation

martong added inline comments. Comment at: test/Analysis/bug_hash_test.cpp:105 +void g() { + TX x; + TX xl; As we discussed, the checking of the equality of the `IssueString` in case of `TX` and `TX` is implicit. And as such it is hard to see that it is

[PATCH] D40937: [clang-tidy] Infinite loop checker

martong added inline comments. Comment at: clang-tidy/misc/InfiniteLoopCheck.cpp:105 + return llvm::make_unique( + *(new ExprSequence(TheCFG.get(), ))); +} `make_unique` is a forwarding function, therefore there is no need to create an object and then

[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D46398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

martong created this revision. martong added reviewers: xazax.hun, a.sidorin. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. `DeclContext` is essentially a list of `Decl`s and a lookup table (`LookupPtr`) but these are encapsulated. E.g. `LookupPtr` is private.

[PATCH] D46398: [ASTImporterTest] Fix potential use-after-free

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:211 StringRef Code; StringRef FileName; std::unique_ptr Unit; Can't we have the same problem with FileName? Perhaps an other alternative would be to make the members

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong updated this revision to Diff 146765. martong added a comment. - Add FIXME Repository: rC Clang https://reviews.llvm.org/D46353 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp

[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs, mgorny. This patch add new tests for structural equivalence. For that a new common header is created which holds the test related language specific types

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

martong added a comment. Hi Aleksei, I am OK with this, I just have a little concern about friend declarations. Since https://reviews.llvm.org/D32947 ( [ASTImporter] FriendDecl importing improvements ) records' structural equivalency depends on the order of their friend declarations. Anyway,

[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl

martong added a comment. In https://reviews.llvm.org/D46958#1101570, @a.sidorin wrote: > So, we fail to add injected name to a CXXRecordDecl that has a described > class template? Yes, we failed to import the implicit `CXXRecordDecl`. Here is how it looked before the fix: From:

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

martong updated this revision to Diff 147274. martong added a comment. Addressing review comments. Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp

[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rL332588: [ASTImporter] Fix missing implict CXXRecordDecl (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46958 Files:

[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC

martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. ClassTemplateSpecialization is put in the wrong DeclContex if implicitly instantiated. This patch fixes it. Repository: rC Clang

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently we do not import the implicit CXXRecordDecl of a ClassTemplateSpecializationDecl. This patch fixes it. Repository: rC Clang

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

martong closed this revision. martong added a comment. Closed by commit: https://reviews.llvm.org/rL332699 Repository: rC Clang https://reviews.llvm.org/D46835 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence

martong updated this revision to Diff 147304. martong marked an inline comment as done. martong added a comment. - Address aleksei's comments Repository: rC Clang https://reviews.llvm.org/D46867 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt

[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence

martong marked 14 inline comments as done. martong added a comment. > Do you plan to enable this functionality for AST import checking? Yes. We'd like to test the structural equivalency independently from ASTImporter, because in certain cases it may have faulty behavior. This can be very handy

[PATCH] D46950: Fix duplicate class template definitions problem

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. We fail to import a `ClassTemplateDecl` if the "To" context already contains a definition and then a forward decl. This is because `localUncachedLookup`

[PATCH] D46958: [ASTImporter] Fix missing implict CXXRecordDecl

martong created this revision. martong added reviewers: xazax.hun, a.sidorin, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Implicit CXXRecordDecl is not added to its DeclContext during import, but in the original AST it is. This patch fixes this. Repository: rC Clang

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong added a comment. Hi Aleksei, Added the FIXME, can you help me with committing this? Repository: rC Clang https://reviews.llvm.org/D46353 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

martong added a comment. Hi Aleksei, Thanks for reviewing this. I could synthesize a test which exercises only the `DeclContext::removeDecl` function. This test causes an assertion without the fix. Removed the rest of the testcases, which are not strictly connected to this change.

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

martong updated this revision to Diff 146845. martong marked 4 inline comments as done. martong added a comment. - Add test for removeDecl, remove unrelated tests Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp

[PATCH] D46835: [ASTImporter] Do not try to remove invisible Decls from DeclContext

martong updated this revision to Diff 146847. martong added a comment. - Remove unrelated CXX14 changes Repository: rC Clang https://reviews.llvm.org/D46835 Files: lib/AST/DeclBase.cpp unittests/AST/ASTImporterTest.cpp unittests/AST/MatchVerifier.h Index:

[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4305 +// Add to the DC only if it was an explicit specialization/instantiation. +if (D2->getTemplateSpecializationKind() != TSK_ImplicitInstantiation) { + LexicalDC->addDeclInternal(D2);

[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC

martong updated this revision to Diff 147966. martong marked an inline comment as done. martong added a comment. Use isExplicitInstantiationOrSpecialization Repository: rC Clang https://reviews.llvm.org/D47058 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index:

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

martong updated this revision to Diff 147994. martong marked an inline comment as done. martong added a comment. - Add new test case for simple CXXRecordDecl - Add comment about ClassTemplateSpecializationDecl implicit CXXRecordDecl Repository: rC Clang https://reviews.llvm.org/D47057

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1962 TagDecl *Definition = D->getDefinition(); - if (Definition && Definition != D) { + if (!D->isImplicit() && Definition && Definition != D) { Decl *ImportedDef = Importer.Import(Definition);

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rL333086: [ASTImporter] Fix missing implict CXXRecordDecl in… (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47057 Files:

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rC333086: [ASTImporter] Fix missing implict CXXRecordDecl in… (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47057?vs=148209=148210#toc Repository: rL LLVM

[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence

martong updated this revision to Diff 148352. martong added a comment. Moved `using std::get` up, before `testStructuralMatch`. Repository: rC Clang https://reviews.llvm.org/D46867 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/CMakeLists.txt unittests/AST/Language.cpp

[PATCH] D46867: [ASTImporter] Add unit tests for structural equivalence

This revision was automatically updated to reflect the committed changes. Closed by commit rC333166: [ASTImporter] Add unit tests for structural equivalence (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D46867?vs=148352=148353#toc Repository: rC

[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC

martong added a comment. > Could you add a test for TSK_Undeclared as well? TLDR: No, I cannot. `TSK_Undeclared` is used to indicate that a template specialization was formed from a template-id but has not yet been declared, defined, or instantiated. Consequently,

[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl

martong added a comment. Looks good to me, but let's wait for Aleksei's approval and comments. Repository: rC Clang https://reviews.llvm.org/D47313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem

martong added a comment. @a.sidorin Yes, that is a very good idea. Just created a new patch which adds that switch for the tests which use the TestBase. https://reviews.llvm.org/D47367 Repository: rC Clang https://reviews.llvm.org/D46950 ___

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase

martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. In order to avoid build failures on MS, we use -fms-compatibility too in the tests which use the TestBase. Repository: rC Clang

[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC

This revision was automatically updated to reflect the committed changes. Closed by commit rL333269: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47058

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase

martong added a comment. This patch does not target `testImport` because I am not sure how to handle the options there: auto RunOptsFrom = getRunOptionsForLanguage(FromLang); auto RunOptsTo = getRunOptionsForLanguage(ToLang); for (const auto : RunOptsFrom) for (const auto :

[PATCH] D47069: [ASTImporter] Enable disabled but passing test

martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. There is a test which passes since https://reviews.llvm.org/D32947, but it was forgotten to be enabled. This patch enables that disabled test.

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Ok, thanks for the explanation. Now it looks good to me. https://reviews.llvm.org/D46940 ___ cfe-commits

[PATCH] D47069: [ASTImporter] Enable disabled but passing test

This revision was automatically updated to reflect the committed changes. Closed by commit rC332728: [ASTImporter] Enable disabled but passing test (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47069?vs=147506=147524#toc Repository: rC Clang

[PATCH] D46940: [ASTImporter] make sure that ACtx::getParents still works

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:6776 + // been invalidated to avoid repeatedly calling this. + ToContext.invalidateParents(); + Can an `Expr` has a parent too? If yes, why not invalidate the parents in `Import(Expr*)` ? I am

[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem

This revision was automatically updated to reflect the committed changes. Closed by commit rC333082: Fix duplicate class template definitions problem (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D46950?vs=148013=148204#toc Repository: rC Clang

[PATCH] D47057: [ASTImporter] Fix missing implict CXXRecordDecl in ClassTemplateSpecializationDecl

martong updated this revision to Diff 148209. martong added a comment. Remove multiline comment and reorder parts of the condition Repository: rC Clang https://reviews.llvm.org/D47057 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index:

[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem

martong updated this revision to Diff 148013. martong marked 5 inline comments as done. martong added a comment. - Addressing Aleksei's comments Repository: rC Clang https://reviews.llvm.org/D46950 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong requested review of this revision. martong added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D47367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. I just wanted to give a detailed justification about why we should import the whole redecl chain. Consider the following code: void f(); // prototype void f() { f(); } Currently, when we import the prototype we end up having two independent functions with

[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl

This revision was automatically updated to reflect the committed changes. Closed by commit rC333522: [ASTImporter] Corrected lookup at import of templated record decl (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47313?vs=148361=149065#toc

[PATCH] D47313: [ASTImporter] Corrected lookup at import of templated record decl

martong added a comment. Balazs, I'll commit it for you in an hour. Repository: rC Clang https://reviews.llvm.org/D47313 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong added a reviewer: balazske. martong added a comment. Balazs, could you please review this patch as well? (This code is not in our fork yet.) Repository: rC Clang https://reviews.llvm.org/D47367 ___ cfe-commits mailing list

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong updated this revision to Diff 149096. martong added a comment. - Remove unused RunOptions typedef and isCXX function Repository: rC Clang https://reviews.llvm.org/D47367 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/Language.cpp unittests/AST/Language.h Index:

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong updated this revision to Diff 149095. martong added a comment. - Remove unused function Repository: rC Clang https://reviews.llvm.org/D47367 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/Language.cpp unittests/AST/Language.h Index: unittests/AST/Language.h

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong updated this revision to Diff 149094. martong added a comment. - Forgot to instantiate some of the parameterized tests Repository: rC Clang https://reviews.llvm.org/D47367 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase

martong updated this revision to Diff 149093. martong added a comment. - Moved the family of `testImport` functions under a test fixture class, so we can use parameterized test. - Refactored `testImport` and `testImportSequence`, because for loops over the different compiler options is no

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong created this revision. martong added reviewers: a.sidorin, balazske, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Generalize the creation of Decl nodes during Import. With this patch we do the same things after and before a new AST node is created

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. > Are you OK with incremental review? Hi Aleksei, thanks for reviewing this! Of course I am OK with an incremental review. Just a minor thing, I'll be back in the office only at the 18th, so I can address all of the comments only then. Repository: rC Clang

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! But let's wait for someone else's review too. @a.sidorin We discovered this error during the CTU analysis of google/protobuf and we could reduce

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

martong added reviewers: akyrtzi, ddunbar. martong added a comment. Adding Argyrios and Daniel as a reviewer for ASTUnit related changes. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

martong added a reviewer: r.stahl. martong added a comment. Adding Rafael too as a reviewer, because he has been working also on the ASTImporter recently. Repository: rC Clang https://reviews.llvm.org/D47450 ___ cfe-commits mailing list

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM! But let's wait the review of those people who have history in `ASTUnit.cpp`. Repository: rC Clang https://reviews.llvm.org/D47445

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong updated this revision to Diff 149107. martong added a comment. - Add a missing "else" Repository: rC Clang https://reviews.llvm.org/D47532 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp lib/AST/DeclBase.cpp test/ASTMerge/class/test.cpp

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun, balazske. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. With this patch when any `FunctionDecl` of a redeclaration chain is imported then we bring in the whole declaration chain. This involves

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

martong created this revision. martong added reviewers: a.sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Add a new test about importing a partial specialization (of a class). Also, this patch adds new tests about the templated-described swing, some of these

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

martong added inline comments. Comment at: test/ASTMerge/injected-class-name-decl-1/Inputs/inject1.cpp:16 +} // namespace google +namespace a { +template class g; balazske wrote: > a.sidorin wrote: > > This looks like raw creduce output. Is there a way to

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:88 + llvm::SmallVector getCanonicalForwardRedeclChain(Decl* D) { +// Currently only FunctionDecl is supported +auto FD = cast(D); balazske wrote: > Assert for FunctionDecl? `cast` itself

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

martong updated this revision to Diff 152660. martong marked 2 inline comments as done. martong added a comment. - Clang format the test code snippet. Repository: rC Clang https://reviews.llvm.org/D47534 Files: unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

This revision was automatically updated to reflect the committed changes. Closed by commit rC335480: [ASTImporter] Import the whole redecl chain of functions (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47532?vs=152691=152693#toc Repository: rC

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

martong updated this revision to Diff 152674. martong added a comment. - Update commit comment and fix broken format in a comment. Repository: rC Clang https://reviews.llvm.org/D47367 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/Language.cpp unittests/AST/Language.h Index:

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong updated this revision to Diff 152691. martong marked 3 inline comments as done. martong added a comment. - Address review comments Repository: rC Clang https://reviews.llvm.org/D47532 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp lib/AST/DeclBase.cpp

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

This revision was automatically updated to reflect the committed changes. Closed by commit rL335455: [ASTImporter] Add new tests about templated-described swing (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47534

[PATCH] D47534: [ASTImporter] Add new tests about templated-described swing

martong added a comment. I addressed the comments, thanks for the review! Repository: rC Clang https://reviews.llvm.org/D47534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47367: [ASTImporter] Add ms compatibility to tests

This revision was automatically updated to reflect the committed changes. Closed by commit rC335464: [ASTImporter] Add ms compatibility to tests which use the TestBase (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47367?vs=152674=152675#toc

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:2021 + +TEST_P(ImportFriendFunctions, + DISABLED_ImportFriendFunctionRedeclChainDefWithClass_ImportTheProto) { a_sidorin wrote: > Could you add comments why these tests are

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1659 + AccessSpecDecl *ToD; + std::tie(ToD, AlreadyImported) = CreateDecl( + D, Importer.getToContext(), D->getAccess(), DC, Loc, ColonLoc); a.sidorin wrote: > As I see, all usage samples

[PATCH] D47450: [ASTImporter] Use InjectedClassNameType at import of templated record.

This revision was automatically updated to reflect the committed changes. Closed by commit rC335600: [ASTImporter] Use InjectedClassNameType at import of templated record. (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D47450?vs=152634=152880#toc

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. The broken lldb tests are fixed with a minor change. We no longer load the Decls from the external source during the call of `DeclContext::containsDecl`. A new function `DeclContext::containsDeclAndLoad` is added which does a load and calls `containsDecl`. Re-apply

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. @labath Sure, looking into it. Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added a comment. This is not trivial to fix. Reverting until we can reproduce and fix it. Reverted with commit: r335491 Repository: rC Clang https://reviews.llvm.org/D47532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D47632: [ASTImporter] Refactor Decl creation

martong updated this revision to Diff 152703. martong added a comment. - Rebase from master. Repository: rC Clang https://reviews.llvm.org/D47632 Files: include/clang/AST/ASTImporter.h include/clang/AST/DeclBase.h lib/AST/ASTImporter.cpp lib/AST/ExternalASTMerger.cpp Index:

[PATCH] D47698: [ASTImporter] import macro source locations

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. This patch is really useful and LGTM! Just found some minor things. Comment at: lib/AST/ASTImporter.cpp:7058 +const SrcMgr::ExpansionInfo = FromSLoc.getExpansion(); +

[PATCH] D48722: [ASTImporter] Update isUsed flag at Decl import.

martong added a comment. > I have a strong feeling of duplication with attribute and flags merging move > in https://reviews.llvm.org/D47632. Maybe it is better to be resolved in that > review by using the same code for attr/flag merging for both newly-created > and mapped decls? Ok, then

[PATCH] D46019: [ASTImporter] Fix isa cast assert

martong added a comment. ping... @xazax.hun could you please help me with the commit? Repository: rC Clang https://reviews.llvm.org/D46019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

martong accepted this revision. martong added a comment. LGTM! Comment at: unittests/AST/ASTImporterTest.cpp:1505 ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); +const internal::VariadicDynCastAllOfMatcher +

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. During import of a class template, lookup may find a forward declaration and structural match falsely reports equivalency in between a fwd decl and a

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong added a comment. > During import of a class template, lookup may find a forward declaration and > structural match falsely reports equivalency in between a fwd decl and a > definition. This can happen when the class to be imported does not have any data members. Structural equivalency

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:1532 +dependentNameType; +TEST(ImportExpr, DependentNameType) { + MatchVerifier Verifier; Perhaps a newline before would make it more independent from the Matcher, as you did it

[PATCH] D46019: [ASTImporter] Fix isa cast assert

martong added a comment. Guys, I still don't have commit rights, could you please help me with the commit? (I think one more good quality patch and then I could request commit rights for myself ...) Repository: rC Clang https://reviews.llvm.org/D46019

[PATCH] D46353: [ASTImporter] Extend lookup logic in class templates

martong added a comment. > should we add this new declaration to the redeclaration chain like we do it > for RecordDecls? I think, on a long term we should. Otherwise we could loose e.g. C++11 attributes which are attached to the forward declaration only. However, I'd do that as a separate

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

martong added a comment. We might get false positives in case of certain substring operations. Consider the case of copying a substring, pseudo code below: const char * s = "abcdefg"; int offset = my_find('d', s); // I want to copy "defg" char *new_subststring = (char*) malloc(strlen(s +

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

martong added a comment. Consider the use of a function pointer: void* malloc(int); int strlen(char*); auto fp = malloc; void bad_malloc(char *str) { char *c = (char *)fp(strlen(str + 1)); } I think, the checker will not match in this case. One might use allocation functions via a

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong updated this revision to Diff 153917. martong marked an inline comment as done. martong added a comment. Remove redundant code and use only StructurlaEquivalence Repository: rC Clang https://reviews.llvm.org/D48773 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp

[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2085 } + } else { +if (!IsStructuralMatch(D, FoundRecord, false)) a_sidorin wrote: > Is it possible to use the added code for the entire condition `if (auto >

[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl

martong added a comment. Just ignore my previous comments, the issue with explicit instantiations could be fixed in a separate independent patch. All is good. Repository: rC Clang https://reviews.llvm.org/D43012 ___ cfe-commits mailing list

[PATCH] D43012: [ASTImporter] Fix lexical DC for templated decls; support VarTemplatePartialSpecDecl

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2858 + + // Templated declarations should never appear in the enclosing DeclContext. + if (!D->getDescribedVarTemplate()) In case of class templates, the explicit instantiation is the member of

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong updated this revision to Diff 152262. martong marked 15 inline comments as done. martong added a comment. - Addressing Alexei's comments. Repository: rC Clang https://reviews.llvm.org/D47532 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp lib/AST/DeclBase.cpp

[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:79 +for (auto R : D->getFirstDecl()->redecls()) { + if (R != D->getFirstDecl()) +Redecls.push_back(R); a.sidorin wrote: > Does this code mean that we can get R == getFirstDecl()

[PATCH] D47946: [ASTmporter] Fix infinite recursion on function import with struct definition in parameters

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. LGTM, just found some minor things. Comment at: unittests/AST/ASTImporterTest.cpp:174 TranslationUnitDecl *TUDecl = nullptr; +

[PATCH] D47698: [ASTImporter] import macro source locations

martong added reviewers: balazske, xazax.hun. martong added subscribers: balazske, xazax.hun. martong added a comment. Adding @balazske and @xazax.hun as reviewers. I think if it gets one more approve then we could merge. I'd like to speed up the things here ... we can't expect Aleksei to

[PATCH] D49840: [AST] Add MatchFinder::matchSubtree

martong added a comment. > MatchFinder::match allows you to match a node. Wrapping your matcher code > with: > auto m = ; > ast_matchers::match(anyOf(m, hashDescendant(m)), node, context); Okay, I understand and accept that. However, I consider that a different level of abstraction.

[PATCH] D49840: [AST] Add MatchFinder::matchSubtree

martong added a comment. > Finder.match also has an overload that takes the node. Can you wrap "Pattern" > above in the anyOf(hasDescendant(...), ...) and match on the node instead of > the full AST? Ok, I changed and wrapped the pattern: template NodeType *match(const Decl *D, const

[PATCH] D49840: [AST] Add MatchFinder::matchSubtree

martong created this revision. martong added reviewers: klimek, aprantl, pcc, sbenza, Prazek, dblaikie, balazske, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Add matchSubtree, so we can traverse on a subtree rooted on a specific node. Currently, we can match **one** node

  1   2   3   4   5   6   7   8   9   10   >