[PATCH] D59692: [ASTImporter] Fix name conflict handling

martong created this revision. martong added reviewers: a_sidorin, shafik, teemperor. Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. martong added a parent revision: D55049: Changed every

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

martong added a reviewer: a_sidorin. martong added a subscriber: a_sidorin. martong added a comment. @shafik, @a_sidorin Ping. Could you please take a look? Guys, this and its child patch are very important patches, because without it the error handling of ASTImporter is not completed. This

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

martong added a comment. > I am aware of similar errors like this with other AST nodes. We had a patch > in our fork to fix that in January > (https://github.com/Ericsson/clang/pull/569/files) I am going to prepare a > patch from that cause I see now this affects you guys in LLDB too. Just

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

martong added a comment. > @martong It's not related to lookup or anything like that, it's just that we > don't have enough information in our debug info AST to allow people to use > meta programming. The custom logic we have in LLDB looks into the std C++ > module to fill out these gaps in

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

martong added a comment. Hi Shafik, Thank you for looking into this. This is indeed a bug, because whenever a we encounter an unnamed EnumDecl in the "from" context then we return with a nameconflict error. However, I think we should fix this differently. First of all, currently

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

martong added a comment. In D59665#1438911 , @a_sidorin wrote: > Hi Shafik, > > Honestly, I was always wondering what does HandleNameConflict actually do. > Its implementation in ASTImporter is trivial and I don't see any of its > overrides in LLDB code

[PATCH] D55049: Changed every use of ASTImporter::Import to Import_New

martong added a reviewer: a_sidorin. martong added a comment. Herald added a subscriber: rnkovacs. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55049/new/ https://reviews.llvm.org/D55049 ___ cfe-commits

[PATCH] D55049: Changed every use of ASTImporter::Import to Import_New

martong added inline comments. Herald added a reviewer: martong. Herald added a project: clang. Comment at: lib/AST/ASTImporter.cpp:7767 + if (!ToDCOrErr) +return ToDCOrErr.takeError(); + auto *ToDC = cast(*ToDCOrErr); Actually, this patch is not merely a

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

martong added a comment. In D59485#1439390 , @teemperor wrote: > > Well, I still don't understand how LLDB synthesis the AST. > > So you have a C++ module in a .pcm file. This means you could create an > > AST from that with ASTReader from it's

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

martong added a comment. In D59485#1439570 , @martong wrote: > In D59485#1439390 , @teemperor wrote: > > > > Well, I still don't understand how LLDB synthesis the AST. > > > So you have a C++ module in a .pcm

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

martong created this revision. martong added reviewers: xazax.hun, a_sidorin, r.stahl. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a project: clang. If CPP dialects are different then return with error. Consider this STL code: template

[PATCH] D57902: [AST] Fix structural inequivalence of operators

martong created this revision. martong added reviewers: shafik, a_sidorin, aaron.ballman. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a project: clang. Operators kind was not checked, so we reported e.g. op- to be equal with op+ Repository: rC

[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. This is to check that operators are handled properly in

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

This revision was automatically updated to reflect the committed changes. Closed by commit rC353425: [ASTImporter] Refactor unittests to be able to parameterize them in a more… (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D57322?vs=184694=185786#toc

[PATCH] D57901: [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. We add a new test to show that redecl chains are not handled properly

[PATCH] D57910: [ASTImporter] Find previous friend function template

martong created this revision. martong added reviewers: a_sidorin, shafik. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. Repository: rC Clang https://reviews.llvm.org/D57910 Files:

[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

This revision was automatically updated to reflect the committed changes. Closed by commit rC353505: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D57905?vs=185924=185925#toc

[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

martong updated this revision to Diff 185924. martong marked an inline comment as done. martong added a comment. - Add comment above ASSERT_NE Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57905/new/ https://reviews.llvm.org/D57905 Files:

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

martong added inline comments. Comment at: lib/CrossTU/CrossTranslationUnit.cpp:232 + + // We do not support CTU across languages (C vs C++). if (LangTo.CPlusPlus != LangFrom.CPlusPlus) { a_sidorin wrote: > The comment change looks strange. Ok, I reverted

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

martong updated this revision to Diff 185926. martong marked 4 inline comments as done. martong added a comment. - Revert comment change - Add lang_dialect_mismatch and stats for that Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57906/new/

[PATCH] D57902: [AST] Fix structural inequivalence of operators

martong marked 2 inline comments as done. martong added inline comments. Comment at: unittests/AST/StructuralEquivalenceTest.cpp:251 + +TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) { + auto t = makeDecls( shafik wrote: > Curious, is this test just for

[PATCH] D57902: [AST] Fix structural inequivalence of operators

This revision was automatically updated to reflect the committed changes. martong marked an inline comment as done. Closed by commit rC353504: [AST] Fix structural inequivalence of operators (authored by martong, committed by ). Changed prior to commit:

[PATCH] D57910: [ASTImporter] Find previous friend function template

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

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

martong added a comment. Herald added a subscriber: cfe-commits. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57590/new/ https://reviews.llvm.org/D57590 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL354259: [ASTImporter] Unify redecl chain tests as type parameterized tests (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL

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

martong updated this revision to Diff 187206. martong marked 2 inline comments as done. martong added a comment. Rebase to master Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57236/new/ https://reviews.llvm.org/D57236 Files: unittests/AST/ASTImporterTest.cpp

[PATCH] D57910: [ASTImporter] Find previous friend function template

martong updated this revision to Diff 187211. martong added a comment. - Add FIXME Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57910/new/ https://reviews.llvm.org/D57910 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index:

[PATCH] D57910: [ASTImporter] Find previous friend function template

This revision was automatically updated to reflect the committed changes. Closed by commit rC354267: [ASTImporter] Find previous friend function template (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D57910?vs=187212=187228#toc Repository: rC Clang

[PATCH] D57910: [ASTImporter] Find previous friend function template

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

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

martong updated this revision to Diff 186445. martong added a comment. Herald added a subscriber: jdoerfert. - Remove numbers when possible - Use disabled tests instead of commented out tests Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57236/new/

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

martong marked 4 inline comments as done. martong added a comment. Thanks for the review Alexei! Comment at: unittests/AST/ASTImporterTest.cpp:3549 + void TypedTest_ImportDefinitionThenPrototype() { +Decl *FromTU0 = getTuDecl(getDefinition(), Lang_CXX, "input0.cc"); +

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

martong updated this revision to Diff 186802. martong added a comment. Rebase to master(trunk) 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/ASTImporter.cpp

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL354027: [ASTImporter] Check visibility/linkage of functions and variables (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL

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

martong added a comment. @shafik Thanks for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57232/new/ https://reviews.llvm.org/D57232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D57740: [ASTImporter] Import every Decl in lambda record

This revision was automatically updated to reflect the committed changes. Closed by commit rL354120: [ASTImporter] Import every Decl in lambda record (authored by martong, committed by ). Herald added subscribers: llvm-commits, jdoerfert. Herald added a project: LLVM. Changed prior to commit:

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

martong added a comment. > How is #if __cplusplus >= 201103L qualitatively different from #ifndef NDEBUG > or #if MYLIB_ABI_VERSION==2 or #if __DATE__ == "2018-04-01"? Ideally, all macros should be the same in the two TUs... If we were very strict then we could check for that, but that might

[PATCH] D57901: [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces

This revision was automatically updated to reflect the committed changes. Closed by commit rL353684: [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL

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

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

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

martong added a comment. Ping. Please raise your objections if you have any until the 18th of February (that date I am going to commit if there are no objections). Also, please let me know if you find this deadline too strict. Repository: rC Clang CHANGES SINCE LAST ACTION

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

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

[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

[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

[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

[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

[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=183307#toc

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

martong updated this revision to Diff 184694. martong marked 2 inline comments as done. martong added a comment. Herald added a project: clang. - Remove unrelated hunks Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57322/new/ https://reviews.llvm.org/D57322

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

martong added a comment. Update: I just removed the unrelated changes. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57322/new/ https://reviews.llvm.org/D57322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

martong updated this revision to Diff 184697. martong added a comment. Herald added a project: clang. - Move hunks into this patch from parent patch Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57232/new/ https://reviews.llvm.org/D57232 Files:

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

martong added a comment. Herald added a project: clang. Ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57236/new/ https://reviews.llvm.org/D57236 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

martong added a comment. > Previously this was not required since all VarDecls were canonical. Not sure > if this change was intended. I did some digging, but am not familiar enough > with the code base to figure out what changed. Does anyone have an idea about > this? We changed the

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

martong added a comment. > I think you should change back to getInit() I am not entirely sure about this because the initalizer may not be attached to the canonical decl. `getInit()` gives the initializer of one given specific Decl, however, `getAnyInitializer()` searches through the whole

[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

[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=183810#toc Repository: rC Clang

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

martong updated this revision to Diff 199629. martong marked 13 inline comments as done. martong added a comment. - Address Shafik's comments and update assertions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44100/new/

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

martong added inline comments. Comment at: lib/AST/ASTImporter.cpp:1643 +if (!ImportedOrErr) { + // For RecordDecls, failed import of a field will break the layout of the + // structure. Handle it as an error. shafik wrote: > What cases are failed

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong updated this revision to Diff 200031. martong added a comment. - Rebase to master - Rebase to D62061 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61333/new/ https://reviews.llvm.org/D61333 Files:

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong marked an inline comment as done. martong added inline comments. Comment at: lldb/source/Symbol/ClangASTImporter.cpp:922 + + Log *log_ast(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_AST)); + if (log_ast) { shafik wrote: > I am going to

[PATCH] D62064: [ASTImporter] Fix unhandled cases in ASTImporterLookupTable

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. In most cases the FriendDecl contains the

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong updated this revision to Diff 200033. martong added a comment. - se -> so Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61333/new/ https://reviews.llvm.org/D61333 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D62066: [ASTImporter] Enable disabled but passing tests

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. Repository: rG LLVM Github Monorepo

[PATCH] D62064: [ASTImporter] Fix unhandled cases in ASTImporterLookupTable

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:4259 static const RecordDecl * getRecordDeclOfFriend(FriendDecl *FD) { QualType Ty = FD->getFriendType()->getType(); + if (auto *Inner =

[PATCH] D62064: [ASTImporter] Fix unhandled cases in ASTImporterLookupTable

martong updated this revision to Diff 204048. martong marked an inline comment as done. martong added a comment. - Simplify getRecordDeclOfFriend Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62064/new/ https://reviews.llvm.org/D62064 Files:

[PATCH] D62064: [ASTImporter] Fix unhandled cases in ASTImporterLookupTable

This revision was automatically updated to reflect the committed changes. Closed by commit rL363062: [ASTImporter] Fix unhandled cases in ASTImporterLookupTable (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong added a comment. http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/29310/ is green. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62373/new/ https://reviews.llvm.org/D62373 ___ cfe-commits mailing list

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:5000 EXPECT_TRUE(ImportedOK); } balazske wrote: > I see now that this test is really not required, the previous test checks the >

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong marked 2 inline comments as done. martong added a comment. @shafik I've been looking for any lldb regression in our Mac machine, could not find any. Now I am looking at http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ . I don't expect regression because here we changed logic

[PATCH] D62373: [ASTImporter] Store import errors for Decls

This revision was automatically updated to reflect the committed changes. Closed by commit rL364279: [ASTImporter] Store import errors for Decls (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D62376: [ASTImporter] Mark erroneous nodes in shared st

martong updated this revision to Diff 206431. martong added a comment. - Set error for FromD if it maps to an existing Decl which has an error set Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62376/new/ https://reviews.llvm.org/D62376 Files:

[PATCH] D62376: [ASTImporter] Mark erroneous nodes in shared st

martong marked 7 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7867 if (PosF != ImportedFromDecls.end()) { -if (LookupTable) +if (SharedState->getLookupTable()) if (auto *ToND = dyn_cast(ToD))

[PATCH] D62376: [ASTImporter] Mark erroneous nodes in shared st

martong updated this revision to Diff 206441. martong marked an inline comment as done. martong added a comment. - Encapsulate by adding addDeclToLookup and removeDeclFromLookup to the shared state Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D62376: [ASTImporter] Mark erroneous nodes in shared st

martong added a reviewer: balazske. martong marked 6 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7830 if (ToD) { +// Already imported (possibly from another TU) and with an error. +if (auto Error =

[PATCH] D62376: [ASTImporter] Mark erroneous nodes in shared st

martong updated this revision to Diff 206429. martong added a comment. - Add FIXMEs - Set error for FromD if it maps to an existing Decl which has an error set - Add test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62376/new/

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong updated this revision to Diff 205093. martong added a comment. - Fix regression of TestFormatters.py Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61333/new/ https://reviews.llvm.org/D61333 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong added a comment. About the regression of TestFormatters.py: I realized that the problem is about the wrong implementation of the ExternalASTSource interface. In the implementation of FindExternalLexicalDecls of this interface, we simply ignored those cases when the given predicate

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong updated this revision to Diff 205814. martong marked an inline comment as done. martong added a comment. - Remove unrelated change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62373/new/ https://reviews.llvm.org/D62373 Files:

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong marked 3 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:5109 } else { // ODR violation. // FIXME HandleNameConflict + return make_error(ImportError::NameConflict); a_sidorin wrote: > Is

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong updated this revision to Diff 206020. martong added a comment. - Remove unused include and using Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62373/new/ https://reviews.llvm.org/D62373 Files: clang/include/clang/AST/ASTImporter.h

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong updated this revision to Diff 206022. martong added a comment. - Add test ErrorPropagatesThroughImportCycles - Change existing test to new behaviour Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62375/new/ https://reviews.llvm.org/D62375

[PATCH] D62376: [ASTImporter] Mark erroneous nodes in shared st

martong added a comment. Alexei, Thanks for the review! I have provided test cases for the 3 previous patches on which this one depends on. I will provide additional tests next week for this one, and of course will address the other comments. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D63603: [ASTImporter] Propagate error from ImportDeclContext

martong updated this revision to Diff 206018. martong added a comment. - Remove formatv b/c it can't handle braces in code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63603/new/ https://reviews.llvm.org/D63603 Files:

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong updated this revision to Diff 206017. martong added a comment. - Remove formatv b/c it can't handle braces in code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62373/new/ https://reviews.llvm.org/D62373 Files:

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong added a comment. Alexei, Thank you for the review, I have added a test which demonstrates the changes. By tracking the import paths and cycles we implement a very strict error handling mechanism, but this seems to be the way to avoid reaching any faulty AST nodes for the ASTImporter

[PATCH] D63603: [ASTImporter] Propagate error from ImportDeclContext

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. martong added a parent revision: D62373:

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong updated this revision to Diff 205811. martong marked 3 inline comments as done. martong added a comment. - Test error values are set for AST nodes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62373/new/ https://reviews.llvm.org/D62373

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong marked 4 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7851 +if (!getImportDeclErrorIfAny(FromD)) { + // Error encountered for the first time. + // After takeError the error is not usable any more in

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong added a comment. Alexei, I think I still owe you some explanation about this patch. I do consider this patch as one of the most intricate patches regarding ASTImporter. I'd like to answer the following questions in this comment: What is an ImportPath and why do we need to track it? What

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong updated this revision to Diff 206244. martong added a comment. - Add back an assertion in setImportDeclError(), remove the condition in Import() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62375/new/ https://reviews.llvm.org/D62375

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8647 - assert(ImportDeclErrors.find(From) == ImportDeclErrors.end() && - "Setting import error allowed only once for a Decl."); ImportDeclErrors[From] =

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong updated this revision to Diff 206218. martong marked 2 inline comments as done. martong added a comment. - Assert that we set the error only once - Remove the macro and use std::string.op+ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D62373: [ASTImporter] Store import errors for Decls

martong marked 4 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7851 +if (!getImportDeclErrorIfAny(FromD)) { + // Error encountered for the first time. + // After takeError the error is not usable any more in

[PATCH] D63603: [ASTImporter] Propagate error from ImportDeclContext

martong updated this revision to Diff 206223. martong added a comment. - Remove the macro and use std::string.op+ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63603/new/ https://reviews.llvm.org/D63603 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong updated this revision to Diff 206228. martong added a comment. - Remove the macro and use std::string.op+ - We may set up an error twice in case of a cycle, thus remove the assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62375/new/

[PATCH] D63603: [ASTImporter] Propagate error from ImportDeclContext

martong updated this revision to Diff 206250. martong marked 2 inline comments as done. martong added a comment. - Use make_scope_exit - Add test ErrorIsNotPropagatedFromMemberToNamespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63603/new/

[PATCH] D63603: [ASTImporter] Propagate error from ImportDeclContext

martong marked 4 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1724 + }; + DefinitionCompleter CompleterRAII(To); jkorous wrote: > You might consider using just a lambda with `llvm::make_scope_exit`. > >

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8668 + +bool ASTImporter::ImportPathTy::hasCycleAtBack() { + return Aux[Nodes.back()] > 1; a_sidorin wrote: > const? Thanks! I made to be const. Comment at:

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong marked an inline comment as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7892 -// Error encountered for the first time. -assert(!getImportDeclErrorIfAny(FromD) && We may set up an error multiple times now, but

[PATCH] D62375: [ASTImporter] Mark erroneous nodes in from ctx

martong updated this revision to Diff 206254. martong marked 5 inline comments as done. martong added a comment. - Use make_scope_exit - Make hasCycleAtBack const Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62375/new/

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

martong updated this revision to Diff 198824. martong added a comment. - Remove remaining FIXMEs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/ https://reviews.llvm.org/D61438 Files: clang/include/clang/AST/ASTImporter.h

[PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

martong updated this revision to Diff 198822. martong marked 3 inline comments as done. martong added a comment. - Remove FIXME and return the error - Use early return where possible and remove redundant else Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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