[PATCH] D43967: [ASTImporter] Add test helper Fixture

martong added a comment. Hi Aleksei, I agree that there are some overlapping functionalities in the patches. As far as I see both make it possible to write tests which import from multiple contexts and I think that is the only redundant part. So, I agree, for now we could use both approaches si

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

martong added a comment. Aleksei, We should definitely try to synchronize our work between Samsung (?) and Ericsson much much more. Unfortunately, it is often we work on the same issue and this can cause delays for both of us. Actually, we solved the same issue in our branch a year ago, see ht

[PATCH] D43967: [ASTImporter] Add test helper Fixture

martong updated this revision to Diff 137563. martong marked 15 inline comments as done. martong added a comment. Fix code review comments. Repository: rC Clang https://reviews.llvm.org/D43967 Files: unittests/AST/ASTImporterTest.cpp unittests/AST/DeclMatcher.h Index: unittests/AST/Decl

[PATCH] D43967: [ASTImporter] Add test helper Fixture

martong added inline comments. Comment at: unittests/AST/ASTImporterTest.cpp:170 +ASTContext &ToCtx = ToAST->getASTContext(); +vfs::OverlayFileSystem *OFS = static_cast( + ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get()); xa

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

[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

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 cfe-commits@li

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

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

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

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

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

[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] 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 (::Create

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

[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 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 &FromEx = FromSLoc.getExpansio

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

[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() i

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

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

martong added a comment. In https://reviews.llvm.org/D47698#1140629, @thakis wrote: > This code is live when reading pchs, correct? Does this have any measurable > perf impact on deserializing pchs for, say, Cocoa.h or Windows.h? No. The deserialization of a PCH is handled in a completely inde

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

martong created this revision. martong added reviewers: a_sidorin, balazske. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: a.sidorin. The crux of the issue that is being fixed is that lookup could not find previous

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

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. When we already have an incomplete underlying type of a typedef in the "To" context, and the "From" context has the same typed

[PATCH] D53697: [Structural Eq] Check for isBeingDefined

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. martong added a dependency: D53693: [ASTImporter] Typedef import brings in the complete type. If one definition is currently being defined, we do not compare for

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

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. FunctionType::ExtInfo holds such properties of a function which are needed mostly for code gen. We should not compare these bi

[PATCH] D53702: [ASTImporter] Set redecl chain of functions before any other import

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. FunctionDecl import starts with a lookup and then we create a new Decl. Then in case of CXXConstructorDecl we further import o

[PATCH] D53704: [ASTImporter] Import overrides before importing the rest of the chain

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. During method import we check for structural eq of two methods. In the structural eq check we check for their isVirtual() flag

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

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

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong created this revision. Herald added subscribers: cfe-commits, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. a_sidorin Repository: rC Clang https://reviews.llvm.org/D53755 Files: include/clang/AST/ASTImporter.h lib/AST/ASTImporter.cpp lib/AST/ExternalASTMerger

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong added a comment. This is about to make `GetAlreadyImportedOrNull` to be a const function, as it should be naturally. This is a query function which should not initiate other imports. Repository: rC Clang https://reviews.llvm.org/D53755

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

martong added a comment. Ping. This is still not committed, however, essential for ctu analyse any C++ project. Aleksei, I am happy to commit it for you if you don't have time, just let me know. Repository: rC Clang https://reviews.llvm.org/D44100

[PATCH] D53704: [ASTImporter] Import overrides before importing the rest of the chain

martong added a comment. Hi Aleksei, Thanks for the review. Yes, we encountered the issue during the analysis of Xerces. Repository: rC Clang https://reviews.llvm.org/D53704 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D53704: [ASTImporter] Import overrides before importing the rest of the chain

This revision was automatically updated to reflect the committed changes. Closed by commit rC345496: [ASTImporter] Import overrides before importing the rest of the chain (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D53704?vs=171097&id=171471#toc Repo

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

martong updated this revision to Diff 171478. martong marked 2 inline comments as done. martong added a comment. - Move FromUT upper, add break Repository: rC Clang https://reviews.llvm.org/D53693 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/AST

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

martong marked 2 inline comments as done. martong added a comment. > I wonder if it is possible to get into situation where non-equivalent decls > are marked equivalent with this patch? If yes, we can create a mapping > between decls being imported and original decls as an alternative solution.

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

martong updated this revision to Diff 171723. martong marked 2 inline comments as done. martong added a comment. - Remove unrelated test Repository: rC Clang https://reviews.llvm.org/D53697 Files: lib/AST/ASTStructuralEquivalence.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/A

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

martong added a comment. Hi Davide, I'd like to draw your attention to an important relation of this patch to a bug in LLDB which manifests as incorrect vtable layouts for LLDB expression code. Please see the conversation started with this message: https://lists.llvm.org/pipermail/cfe-dev/2018-

[PATCH] D56581: [ASTImporter] Set the described template if not set

martong created this revision. martong added reviewers: shafik, a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. During the addition of an injected class type to a record it may happen that a CXXRecordDecl in the redecl c

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong added a comment. Hi Shafik, thank you for this patch! Generally it looks quite okay to me, but I have a few minor comments. Comment at: lib/AST/ASTImporter.cpp:3049 + // Import the body of D and attach that to FoundByLookup. + // This should probably

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong added a comment. > @martong the only issue is that I am seeing a regression on > Analysis/ctu-main.cpp when I run check-clang. I am going to look into it but > if you have any insights that would be helpful. I am looking into it and will come back with what I have found. CHANGES SINCE

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong added a comment. I have found some other minor things in the tests. Comment at: unittests/AST/ASTImporterTest.cpp:2275 + auto DFDef = cxxMethodDecl( + hasName("f"), hasParent(cxxRecordDecl(hasName("B"))), isDefinition()); + auto FDefAll = cxxMethodDecl(hasName("f

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong added a comment. Shafik, I have realized what's the problem with the `ctu-main` test. When we import the body and set the new body to the existing FunctionDecl then the parameters are still the old parameters so the new body does not refer to the formal parameters! This way the analyzer

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:3046 +if (!D->doesThisDeclarationHaveABody()) + return cast(const_cast(FoundByLookup)); +else { balazske wrote: > The `cast` should not be needed here (and is not done at

[PATCH] D56651: [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl

martong accepted this revision. martong added a comment. The updated version looks good to me! Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56651/new/ https://reviews.llvm.org/D56651 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong added a comment. > So the problem is that there are references to ParmVarDecl from inside > function body and at import of ParmVarDecl always a new one is created even > if there is an already existing (in the existing function prototype)? Yes. During the import of the body we import a

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC352050: [ASTImporter] Fix inequality of functions with different attributes (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D53699?vs=178060&id=183307#toc Reposi

[PATCH] D56936: Fix handling of overriden methods during ASTImport

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Shafik, thanks for addressing the comments. This looks good to me now! Comment at: lib/AST/ASTImporter.cpp:3042 + if (FoundByLookup) { +if (auto *MD = dyn_cast(Found

[PATCH] D56581: [ASTImporter] Set the described template if not set

martong added a comment. @shafik Ok, I'll take a look into the regression soon. What are exactly gmodules? Is it macOS only? Do you have a description or some docs about it? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56581/new/ https://reviews.llvm.org/D56581

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

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. During import of a global variable with external visibility the lookup will find variables (with the same na

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

martong updated this revision to Diff 183523. martong added a comment. - Remove old style import in case of FoundArray Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57232/new/ https://reviews.llvm.org/D57232 Files: include/clang/AST/ASTImporter.h lib/AST/AST

[PATCH] D57235: [AST] Add structural eq tests for template args

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. New tests added to verify equivalency of templates when their parameters are different. Repository: rC Clang https://reviews.llvm.org/D5723

[PATCH] D57236: [ASTImporter] Unify redecl chain tests as type parameterized tests

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. This patch unifies all those tests which check the correctness of the redecl chains. Previously we had sever

[PATCH] D57235: [AST] Add structural eq tests for template args

This revision was automatically updated to reflect the committed changes. Closed by commit rC352345: [AST] Add structural eq tests for template args (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D57235?vs=183525&id=183810#toc Repository: rC Clang CH

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

martong updated this revision to Diff 183826. martong marked 5 inline comments as done. martong added a comment. - Remove dumpDeclContext() call - Remove superfluous else Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57232/new/ https://reviews.llvm.org/D57232 Fi

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

martong marked an inline comment as done. martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:2954 +return Found->hasExternalFormalLinkage(); + else if (Importer.GetFromTU(Found) == From->getTranslationUnitDecl()) { +if (From->isInAnonymousNamespace()) ---

[PATCH] D57322: [ASTImporter] Refactor unittests to be able to parameterize them in a more flexible way

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. Currently `TestImportBase` is derived from `ParameterizedTestsFixture` which explicitly states that the gtes

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

martong updated this revision to Diff 183834. martong added a comment. I have created a separate patch for the test related refactor, this patch now depends on that patch and contains merely the visibility related change and their tests. Repository: rC Clang CHANGES SINCE LAST ACTION https:

[PATCH] D56581: [ASTImporter] Set the described template if not set

martong added a comment. > @martong have you had a chance to look at this some more? We ran into another > problem that this fixes partially. I didn't have time to deal with this yet (these days we are preparing an internal release and that took away my time). > Can I help in any way? I think

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

This revision was automatically updated to reflect the committed changes. Closed by commit rC345760: [ASTImporter][Structural Eq] Check for isBeingDefined (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D53697?vs=171723&id=171984#toc Repository: rC Cla

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

martong added a comment. Herald added a reviewer: shafik. Shafik, Sorry for the inconvenience. Seems like the libcxx tests of lldb are automatically skipped on Linux, so it slipped through my test execution. Also, I did not receive any email from this build server about the build failure. Never

[PATCH] D53979: [analyzer][CTU] Correctly signal in the function index generation tool if there was an error

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Looks good to me. It is great to see that we can get rid of so many header and lib dependencies. https://reviews.llvm.org/D53979 ___ cfe-commi

[PATCH] D53702: [ASTImporter] Set redecl chain of functions before any other import

martong added a comment. > Did you ever resolve the issue of libcxx tests not running > https://reviews.llvm.org/D53697 Hi @shafik , Sorry for the late reaction, I was on a two weeks long vacation recently. My priority is to make those libcxx tests pass, I did not forget. :) Repository: rC C

[PATCH] D53702: [ASTImporter] Set redecl chain of functions before any other import

martong added a comment. > Did you ever resolve the issue of libcxx tests not running > https://reviews.llvm.org/D53697 Oh, by the way this change is completely independent from that other patch. Repository: rC Clang https://reviews.llvm.org/D53702 ___

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

martong added a subscriber: a_sidorin. martong added a comment. > I think these changes make sense at a high level but I am not sure about the > refactoring strategy. I am especially concerned we may end up in place where > all the effected users of the API don't get updated and we are stuck wit

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

martong added a comment. Herald added a subscriber: gamesh411. @shafik, I could run the libcxx tests on my Linux machine (Ubuntu 16.04) by installing libcxx: `sudo apt-get install libc++-dev`. From the logs of your build server seems like the crash happened with a `_gmodules` test. Unfortunatel

[PATCH] D53702: [ASTImporter] Set redecl chain of functions before any other import

This revision was automatically updated to reflect the committed changes. Closed by commit rC347306: [ASTImporter] Set redecl chain of functions before any other import (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D53702?vs=171093&id=174763#toc Reposi

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

martong updated this revision to Diff 174893. martong marked 4 inline comments as done. martong added a comment. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411. - Minor style changes and rename a function Repository: rC Clang https://reviews.llvm.org/D53655 Files: in

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

martong added inline comments. Comment at: lib/AST/DeclBase.cpp:1469 assert(Pos != Map->end() && "no lookup entry for decl"); -if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND) +// Remove the decl only if it is contained. +if ((Pos->

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong updated this revision to Diff 174897. martong marked an inline comment as done. martong removed a reviewer: shafik. martong removed a subscriber: gamesh411. martong added a comment. Herald added a reviewer: shafik. - Better style without braces Repository: rC Clang https://reviews.llv

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong marked an inline comment as done. martong added inline comments. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411. Comment at: lib/AST/ASTImporter.cpp:7716 } } balazske wrote: > This can be simplified by removing brace character

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

martong added a comment. Herald added a subscriber: gamesh411. That's a good point. I agree, we should check some bits (calling convention bits) but not all (e.g. noreturn bit). I am going to create another patch which replaces this. Repository: rC Clang https://reviews.llvm.org/D53699 _

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

martong updated this revision to Diff 175058. martong added a comment. - Revert "Minor style changes and rename a function" Repository: rC Clang https://reviews.llvm.org/D53655 Files: include/clang/AST/DeclContextInternals.h include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.c

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

martong marked 3 inline comments as done. martong added inline comments. Comment at: lib/AST/DeclBase.cpp:1469 assert(Pos != Map->end() && "no lookup entry for decl"); -if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND) +// Remove the decl on

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

martong added inline comments. Herald added a reviewer: shafik. Herald added a subscriber: gamesh411. Comment at: lib/AST/ASTImporter.cpp:2310 +D->getUnderlyingType(), FoundTypedef->getUnderlyingType())) { + QualType FromUT = D->getUnderlyingType(); +

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

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

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] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong added a subscriber: gamesh411. martong added a comment. @shafik , @gamesh411 I think when I used arcanist to update the patch it removed you as a reviewer and a subscriber. I am really sorry about this. Fortunately the Herald script added you guys back. And once you are here, this is a g

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

martong updated this revision to Diff 175091. martong marked an inline comment as done. martong added a comment. - Keep the good changes and use the name 'containsInVector' Repository: rC Clang https://reviews.llvm.org/D53655 Files: include/clang/AST/DeclContextInternals.h include/clang/

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

martong added a comment. > I see that now everything is reverted, the "good" things too (change to > indirectFieldDecl and a line split)? Yes, you are completely right, sorry for this mess. I just have updated so the good things remain. Repository: rC Clang https://reviews.llvm.org/D53655

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

martong updated this revision to Diff 175127. martong added a comment. - Use MostRecentDecl when setting PrevDecl Repository: rC Clang https://reviews.llvm.org/D53655 Files: include/clang/AST/DeclContextInternals.h include/clang/ASTMatchers/ASTMatchers.h lib/AST/ASTImporter.cpp lib/A

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

martong added a comment. This will break LLDB, unless https://reviews.llvm.org/D54863 is applied. @shafik Could you please take a look on https://reviews.llvm.org/D54863 ? Repository: rC Clang https://reviews.llvm.org/D53655 ___ cfe-commits mail

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

martong added a comment. In D53818#1307321 , @a_sidorin wrote: > LGTM. Thank you for addressing my questions! Hi Alexei, Could you please also take a look on that patch which this one depends on? https://reviews.llvm.org/D53751 I think the changes of t

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

martong updated this revision to Diff 175241. martong added a comment. - Get data of CXXRecordDecl only if set Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53697/new/ https://reviews.llvm.org/D53697 Files: lib/AST/ASTImporter.cpp lib/AST/ASTStructuralEquiva

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

martong added a comment. I have reproduced the issue on MacOS, fixed it and updated the patch accordingly. Soon I will commit and will monitor if any build bots are being broken. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53697/new/ https://reviews.llvm.org/D

[PATCH] D53697: [ASTImporter][Structural Eq] Check for isBeingDefined

This revision was automatically updated to reflect the committed changes. Closed by commit rL347564: [ASTImporter][Structural Eq] Check for isBeingDefined (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM CHANGES SINCE LAST ACTION https://revi

[PATCH] D54898: Set MustBuildLookupTable on PrimaryContext in ExternalASTMerger

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM. Thank you! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54898/new/ https://reviews.llvm.org/D54898 ___

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL347648: [ASTImporter] Typedef import brings in the complete type (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53693?v

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong marked 2 inline comments as done. martong added inline comments. Comment at: lib/AST/ExternalASTMerger.cpp:154 ToContainer->setHasExternalLexicalStorage(); - ToContainer->setMustBuildLookupTable(); + ToContainer->getPrimaryContext()->setMustBuildLookupTab

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

martong marked 20 inline comments as done. martong added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:1169 +/// matches 'a'. +extern const internal::VariadicDynCastAllOfMatcher +indirectFieldDecl; aaron.ballman wrote: > Be sure to up

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

martong updated this revision to Diff 175482. martong marked 5 inline comments as done. martong added a comment. - Address several minor review comments Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53655/new/ https://reviews.llvm.org/D53655 Files: docs/LibAST

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

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

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] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong added a subscriber: teemperor. martong added a comment. Hi @shafik , Thank you for your review. I have removed the call of `getPrimaryContext`. Fortunately, we already have one patch for the `getPrimaryContext` related changes, made by @teemperor : https://reviews.llvm.org/D54898 . I h

[PATCH] D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull

martong updated this revision to Diff 175639. martong marked an inline comment as done. martong added a comment. - Remove call of 'getPrimaryContext' Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53755/new/ https://reviews.llvm.org/D53755 Files: include/clang/

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

martong marked 4 inline comments as done. martong added inline comments. Comment at: include/clang/AST/DeclContextInternals.h:125-129 + bool containsInVector(const NamedDecl *D) { +assert(getAsVector() && "Must have a vector at this point"); +DeclsTy &Vec = *getAsVector(

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

martong updated this revision to Diff 175641. martong marked 2 inline comments as done. martong added a comment. - Remove containsInVector Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53655/new/ https://reviews.llvm.org/D53655 Files: docs/LibASTMatchersRefere

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

martong added a comment. @aaron.ballman Thanks for your review. I have updated the patch to remove `containsInVector`. > This seems mostly reasonable to me, but @rsmith is more well-versed in this > area and may have further comments. You should give him a few days to chime > in before committ

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