[PATCH] D49792: [ASTmporter] SourceRange-free function parameter checking for declarations

This revision was automatically updated to reflect the committed changes. Closed by commit rL339018: [ASTmporter] SourceRange-free function parameter checking for declarations (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews

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

martong added a comment. Ping. Manuel, I still don't see how could we apply `match(anyOf(node), hasDescendant(node))` to the problem of general subtree traversal. (I'd like to have support for not just `decl()` but other kind of nodes too.) Could you please advise how to move on? Also, could yo

[PATCH] D50428: Add support for importing imaginary literals

martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Repository: rC Clang https://reviews.llvm.org/D50428 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTM

[PATCH] D50428: Add support for importing imaginary literals

martong updated this revision to Diff 159662. martong added a comment. Remove superflous newline Repository: rC Clang https://reviews.llvm.org/D50428 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/ASTImport

[PATCH] D50444: Fix structural inequivalency of forward EnumDecl

martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently we consider one forward declared RecordDecl and another with a definition equal. We have to do the same in case of enums. Repository: rC Cla

[PATCH] D50451: Fix import of class templates partial specialization

martong created this revision. martong added reviewers: a_sidorin, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently there are several issues with the import of class template specializations. (1) Different TUs may have class template specializations with the

[PATCH] D50428: [ASTImporter] Add support for importing imaginary literals

This revision was automatically updated to reflect the committed changes. Closed by commit rL339334: Add support for importing imaginary literals (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50428?vs=159662&id=

[PATCH] D50444: [ASTImporter] Fix structural inequivalency of forward EnumDecl

This revision was automatically updated to reflect the committed changes. Closed by commit rL339336: Fix structural inequivalency of forward EnumDecl (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50444?vs=159697

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong marked 5 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2872 Importer.MapImported(D, FoundField); +// In case of a FieldDecl of a ClassTemplateSpecializationDecl, the +// initializer of a FieldDecl migh

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong marked 3 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4551 + for (auto *FromField : D->fields()) +Importer.Import(FromField); + martong wrote: > a_sidorin wrote: > > The result of import is unche

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong updated this revision to Diff 160552. martong added a comment. - Add new test - Fix indentation Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInternal.cpp unittests/AST/AS

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4550 + // in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) +Importer.Import(FromField); martong wrote: > a_sidorin wrote: > > Importing a

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

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1317 + for (auto *D : FromRD->decls()) { +Decl *ToD = Importer.GetAlreadyImportedOrNull(D); +assert(ToRD == ToD->getDeclContext() && ToRD->containsDecl(ToD)); Is it sure that `ToD` will n

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

martong added a comment. Hi Aleksei, Thank you for this patch. With Balazs, we are working on something similar, but with a different, fine grained error value mechanism. Unfortunately we were not aware of that you have been working on error handling, and we didn't say that we are working on er

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

martong added a comment. > Do you think we can hold back this patch for a few days until Balazs prepares > the Expected based version? Then I think we could compare the patches and > we could merge the best from the two of them. I mean, once Balazs is also ready, then we can create a combined p

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

martong added a comment. Roughly, this is the direction where we are heading with `Expected`: https://github.com/Ericsson/clang/pull/457/commits/783b7f5cce9de589a5c3c3ae983398cf499077ec (If you are interested, here is our whole thought process: https://github.com/Ericsson/clang/pull/457) Reposi

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

martong accepted this revision. martong added inline comments. This revision is now accepted and ready to land. Comment at: lib/AST/ASTImporter.cpp:1317 + for (auto *D : FromRD->decls()) { +Decl *ToD = Importer.GetAlreadyImportedOrNull(D); +assert(ToRD == ToD->getDeclCon

[PATCH] D50792: [ASTImporter] Add test for member pointer types.

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

[PATCH] D50793: [ASTImporter] Add test for importing CompoundAssignOperators

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Herald added a subscriber: rnkovacs. Looks good. Repository: rC Clang https://reviews.llvm.org/D50793 ___ cfe-commits mailing list cfe-commi

[PATCH] D50810: [ASTImporter] Add test for DoStmt

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

[PATCH] D50811: [ASTImporter] Add test for WhileStmt

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

[PATCH] D50813: [ASTImporter] Add test for IndirectGotoStmt

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

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

martong added a comment. > Yes, I was thinking about it too. The reason why I chose Optional was that > ASTImporter clients don't use the error kind. If you have any plans on > changing this, Expected is a preferred approach. Yes, we plan to submit a patch to LLDB too which would apply the use

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong updated this revision to Diff 161698. martong marked an inline comment as done. martong added a comment. - Change comments Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatchersInte

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong marked an inline comment as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:4550 + // in the "From" context, but not in the "To" context. + for (auto *FromField : D->fields()) +Importer.Import(FromField); a_sidor

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

martong updated this revision to Diff 161699. martong added a comment. - Add one more TODO to import instantiated exception specifications Repository: rC Clang https://reviews.llvm.org/D50451 Files: include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/ASTMatchers/ASTMatc

[PATCH] D51001: [ASTImporter] Add test for CXXForRangeStmt

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

[PATCH] D50451: [ASTImporter] Fix import of class templates partial specialization

This revision was automatically updated to reflect the committed changes. Closed by commit rL340402: Fix import of class templates partial specialization (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D50451?vs=16

[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 http://lists.llvm.org/cgi

[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. `DeclContext::removeDecl`

[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] 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 http://lists.llvm.org/cgi-bin/mailman/listin

[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] 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 unittest

[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: unittests/AST/MatchVerifier.

[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` d

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

[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, I

[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

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

[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: cfe/

[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] 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 unittests/AST/Languag

[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 http://lists.llvm.org/cgi-bin/

[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 https://reviews

[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 https://reviews.llvm

[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. Reposito

[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] 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 m

[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&id=147524#toc Repository: rC Clang htt

[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: unit

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

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

[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 unittests/AST/DeclMatch

[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&id=148204#toc Repository: rC Clang h

[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: unittests/AST/ASTImporterTest.

[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&id=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 unitte

[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&id=148353#toc Repository: rC C

[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 http://lists.llvm.org/cgi-bin/mailman/list

[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, `ClassTemplateSpecializationDe

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

[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 https://reviews.llvm.

[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 ___ cf

[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 &FromArgs : RunOptsFrom) for (const auto &To

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

[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 cfe-comm

[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 cfe-commits@lists.llvm.org

[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] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase

martong added a comment. > I agree with that. I think we need to test just import pairs > {/*From*/no_option, /*To*/no_option}, {option_1, option1}, {option_2, > option_2}, ...{option_n, option_n}. This patch does exactly that with the parameterized tests. Each elements of the `DefaultTestValu

[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] 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&id=149065#toc Reposito

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

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

[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] D51533: [ASTImporter] Merge ExprBits

martong created this revision. martong added reviewers: a_sidorin, r.stahl, xazax.hun. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Some `Expr` classes set up default values for the `ExprBits` of `Stmt`. These default values are then overwritten by

[PATCH] D51533: [ASTImporter] Merge ExprBits

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:3241 + auto *ToD = Import(FromD, Lang_CXX11); + ASSERT_TRUE(ToD); + auto *ToInitExpr = cast(ToD)->getAnyInitializer(); a_sidorin wrote: > EXPECT_TRUE (same below). Thanks, changed it.

[PATCH] D51533: [ASTImporter] Merge ExprBits

This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rL341316: [ASTImporter] Merge ExprBits (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llv

[PATCH] D51597: [ASTImporter] Fix import of VarDecl init

martong created this revision. martong added reviewers: a_sidorin, xazax.hun, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: a.sidorin. The init expression of a VarDecl is overwritten in the "To" context if we import a

[PATCH] D51597: [ASTImporter] Fix import of VarDecl init

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:3763 +INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportImplicitMethods, +DefaultTestValuesForRunOptions, ); This hunk has nothing to do with this change, but p

[PATCH] D50672: [ASTImporter] Change the return result of Decl import to Optional

martong added a comment. Here is the `Expected` based patch: https://reviews.llvm.org/D51633 Repository: rC Clang https://reviews.llvm.org/D50672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D51633: [ASTImporter] Added error handling for AST import.

martong added a comment. This patch is huge, but we change here almost all implementation functions of `ASTNodeImporter` to return with either `Error` or with `Expected`. We could not really come up with a cohesive but smaller patch because of the recursive nature of the importer. (We are open t

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

martong added a comment. From this little information I have hear are my thoughts: > match callExpr(allOf (callee(functionDecl(hasName("std::sort"))), > hasArgument(0, > hasDescendant(declRefExpr(to(fieldDecl(hasName("value_type")) I think this is a good direction, but keep

[PATCH] D51597: [ASTImporter] Fix import of VarDecl init

martong marked 3 inline comments as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1441 + To->setInit(ToInit); + if (From->isInitKnownICE()) { +EvaluatedStmt *Eval = To->ensureEvaluatedStmt(); a_sidorin wrote: > I see that this is

[PATCH] D51597: [ASTImporter] Fix import of VarDecl init

martong updated this revision to Diff 165039. martong marked an inline comment as done. martong added a comment. - Fix formatting and typo Repository: rC Clang https://reviews.llvm.org/D51597 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImport

[PATCH] D49223: [AST] Check described template at structural equivalence check.

martong added a reviewer: a_sidorin. martong added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D49223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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

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

martong added a comment. > Usually we use match(anyOf(node), hasDescendant(node)). Or did I > misunderstand what you want? My understanding is that, the free function template `match` uses `MatchFinder::matchAST`, which will start the traverse from the TranslationUnitDecl. And there is no opti

[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. `ast_matc

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

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

martong added a comment. > If you know the node is a decl, wrapping it in decl() should be enough. > Does this work? > auto WrappedMatcher = decl(anyOf(...)); Unfortunately this gives again the private ctor error (the same error when I called explicitly the overload with the `DeclarationMatche

[PATCH] D49223: [AST] Check described template at structural equivalence check.

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/D49223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi

[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] 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 cfe-

[PATCH] D43967: Add test helper Fixture

martong created this revision. martong added reviewers: a.sidorin, xazax.hun, szepet. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. [ASTImporter] Add a helper test Fixture, so we can add tests which may check internal attributes of AST nodes. Also it makes possible to import from severa

[PATCH] D43967: Add test helper Fixture

martong added a comment. We add several test cases here, some of them are disabled. We plan to pass the disabled tests in different patches. Repository: rC Clang https://reviews.llvm.org/D43967 ___ cfe-commits mailing list cfe-commits@lists.llvm.

  1   2   3   4   5   6   7   8   9   10   >