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

martong marked 8 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:5039 + if (!ToOrErr) +// FIXME: return the error? +consumeError(ToOrErr.takeError()); aprantl wrote: > We don't

[PATCH] D61786: [ASTImporter] Separate unittest files

This revision was automatically updated to reflect the committed changes. Closed by commit rC360572: [ASTImporter] Separate unittest files (authored by martong, committed by ). Changed prior to commit: https://reviews.llvm.org/D61786?vs=199225=199226#toc Repository: rC Clang CHANGES SINCE

[PATCH] D61790: [C++20] add consteval specifier

martong resigned from this revision. martong added a comment. Herald added a subscriber: rnkovacs. ASTImporter.cpp looks good to me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61790/new/ https://reviews.llvm.org/D61790

[PATCH] D61786: [ASTImporter] Separate unittest files

martong marked 8 inline comments as done. martong added a comment. Thanks for the review Alexei! Comment at: clang/unittests/AST/ASTImporterGenericRedeclTest.cpp:20 + +// FIXME put these structs and the tests rely on them into their own separate +// test file!

[PATCH] D61786: [ASTImporter] Separate unittest files

martong updated this revision to Diff 199225. martong marked 2 inline comments as done. martong added a comment. - Address Alexei's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61786/new/ https://reviews.llvm.org/D61786 Files:

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

martong added a comment. @aprantl Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/ https://reviews.llvm.org/D61438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

martong marked an inline comment as done. martong added inline comments. Comment at: clang/include/clang/AST/ASTImporter.h:203 /// context, or the import error. -llvm::Expected Import_New(TypeSourceInfo *FromTSI); -// FIXME: Remove this version. -TypeSourceInfo

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

martong added a comment. The `ASTImporter.cpp` looks good to me. (Becasue the `BranchHint` is a simple an enum, so we don't need to specifically import that as we would in case of e.g. an `Expr`.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59467/new/

[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

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 project: clang. Structural equivalence did not handle dependent template args properly when the arg

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

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. We add a new member which is a mapping from the

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

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. During import of a specific Decl D, it may happen

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

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. This is the third step of the full error handling.

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL361139: [ASTImporter] Enable disabled but passing tests (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter

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. Currently ASTImporter::NonEquivalentDecls member

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

martong marked an inline comment as done. martong added inline comments. Comment at: clang/include/clang/AST/ASTImporter.h:203 /// context, or the import error. -llvm::Expected Import_New(TypeSourceInfo *FromTSI); -// FIXME: Remove this version. -TypeSourceInfo

[PATCH] D61786: [ASTImporter] Separate unittest files

martong created this revision. martong added a reviewer: a_sidorin. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs, mgorny. Herald added a reviewer: a.sidorin. Herald added a reviewer: shafik. Herald added a project: clang. Move generic redecl chain tests and

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

martong marked 2 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 200435. martong marked an inline comment as done. martong added a comment. - Remove getUnderlyingType Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62064/new/ https://reviews.llvm.org/D62064 Files:

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL360760: [ASTImporter] Use llvm::Expected and Error in the importer API (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

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

martong updated this revision to Diff 199569. martong added a comment. - Rebase to master 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 added a comment. Thank you guys for the review! In D61438#1501457 , @aprantl wrote: > Alright, thanks for taking the time to walk me through the thought process! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

martong marked 2 inline comments as done. martong added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c:8 int b; -} FILE; +} MYFILE; In TestCmodules.py we have `import Darwin` and then `expr *fopen("/dev/zero",

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

martong created this revision. martong added reviewers: shafik, teemperor, jingham, clayborg, a_sidorin. Herald added subscribers: lldb-commits, cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added projects: clang, LLDB. With LLDB we use

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. ASTImporter.cpp and ASTStructuralEquivalence.cpp looks good to me now. I am resigning as a reviewer since I don't feel competent enough to review the rest of the change. CHANGES SINCE

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

martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:625 + "class shouldNotBeImported {};", Lang_CXX, "class realDecl {};", Lang_CXX, + "shouldNotBeImported", RedirectingImporter::Constructor); + auto *Imported = cast(To);

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

martong accepted this revision. martong added a comment. Looks good, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59485/new/ https://reviews.llvm.org/D59485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

martong created this revision. martong added reviewers: shafik, teemperor, aprantl, a_sidorin, balazske. Herald added subscribers: lldb-commits, cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added projects: clang, LLDB. This is the final phase of

[PATCH] D61424: [ASTImporter] Fix inequivalence of unresolved exception spec

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. Structural equivalence of methods can falsely report false when the

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

martong updated this revision to Diff 197948. martong added a comment. - Log and do not assert anywhere 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] D59692: [ASTImporter] Fix name conflict handling

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

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

martong added a reviewer: xazax.hun. martong added a comment. @xazax.hun Could you please take a look? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59798/new/ https://reviews.llvm.org/D59798 ___ cfe-commits mailing

[PATCH] D61424: [ASTImporter] Fix inequivalence of unresolved exception spec

martong updated this revision to Diff 198626. martong marked 2 inline comments as done. martong added a comment. - Improve and fix typo in comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61424/new/ https://reviews.llvm.org/D61424 Files:

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

martong updated this revision to Diff 198623. martong added a comment. - Use LLDB_LOG_ERROR in ImportDefinitionTo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/ https://reviews.llvm.org/D61438 Files:

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

martong updated this revision to Diff 198618. martong added a comment. - Add braces to 'true' cases when 'false' case has braces - Simplify logging and error handling with LLDB_LOG_ERROR Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61438/new/

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

martong added a comment. Ping @shafik @teemperor Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61333/new/ https://reviews.llvm.org/D61333 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D61424: [ASTImporter] Fix inequivalence of unresolved exception spec

This revision was automatically updated to reflect the committed changes. Closed by commit rL360261: [ASTImporter] Fix inequivalence of unresolved exception spec (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D63920: [CTU] Add support for virtual functions

martong updated this revision to Diff 207738. martong added a comment. - Use -analyzer-config ipa=inlining in the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63920/new/ https://reviews.llvm.org/D63920 Files:

[PATCH] D63920: [CTU] Add support for virtual functions

martong added a comment. In D63920#1566035 , @baloghadamsoftware wrote: > Try to set analyzer option `IPAMode` to something different from its default > value which is `dynamic-bifurcate` in the test file. Ok I set it to `ipa=inlining` and then we

[PATCH] D59798: [analyzer] Add analyzer option to limit the number of imported TUs

martong accepted this revision. martong added inline comments. This revision is now accepted and ready to land. Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:297 +"for import when inlining functions during CTU analysis. " +

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

martong updated this revision to Diff 207774. martong marked an inline comment as done. martong added a comment. - Add check for isLambda() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64075/new/ https://reviews.llvm.org/D64075 Files:

[PATCH] D63920: [CTU] Add support for virtual functions

martong added a comment. In D63920#1568049 , @Szelethus wrote: > In D63920#1568031 , @martong wrote: > > > In D63920#1566035 , > > @baloghadamsoftware wrote: > > > > > Try

[PATCH] D64073: [ASTImporter] Fix import of lambda in function param

martong updated this revision to Diff 207776. martong marked an inline comment as done. martong added a comment. - 'the ' -> 'the ' - Use std::distance() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64073/new/ https://reviews.llvm.org/D64073

[PATCH] D64073: [ASTImporter] Fix import of lambda in function param

martong marked 3 inline comments as done. martong added a comment. Thanks for the review Alexei! Comment at: clang/unittests/AST/ASTImporterTest.cpp:5103 + // count. + for (auto : ToL->decls()) { +(void)D; a_sidorin wrote: > Can we use std::distance

[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

martong marked 2 inline comments as done. martong added inline comments. Comment at: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp:173 +DE2->getQualifier()); + } else if (auto CastE1 = dyn_cast(E1)) { +auto *CastE2 = dyn_cast(E2);

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1182 + if (D1CXX->isLambda()) { +if (!D2CXX->isLambda()) a_sidorin wrote: > Should we return false if

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:8503 +llvm::Expected ASTImporter::Import(const APValue ) { + APValue Result; Tyker wrote: > martong wrote: > > Tyker wrote: > > > martong wrote: > > > > Looks okay, but could we have

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

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 project: clang. We falsely state inequivalence if the template parameter is a

[PATCH] D63920: [CTU] Add support for virtual functions

This revision was automatically updated to reflect the committed changes. Closed by commit rL365133: [CTU] Add support for virtual functions (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D63878: [CTU] Add missing statistics

martong created this revision. martong added a reviewer: xazax.hun. Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D63878 Files:

[PATCH] D63920: [CTU] Add support for virtual functions

martong marked an inline comment as done. martong added inline comments. Comment at: clang/test/Analysis/ctu-main.cpp:112 + // We cannot decide about the dynamic type. + clang_analyzer_eval(obj->fvcl(1) == 8); // expected-warning{{FALSE}} expected-warning{{TRUE}} +

[PATCH] D63878: [CTU] Add missing statistics

This revision was automatically updated to reflect the committed changes. Closed by commit rL364630: [CTU] Add missing statistics (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D63920: [CTU] Add support for virtual functions

martong created this revision. martong added reviewers: Szelethus, xazax.hun. Herald added subscribers: cfe-commits, gamesh411, dkrupp, rnkovacs. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D63920 Files:

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

martong added a comment. @a_sidorin Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63603/new/ https://reviews.llvm.org/D63603 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

martong updated this revision to Diff 207063. martong marked 4 inline comments as done. martong added a comment. - Add further checks for the DeclarationName overload Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62329/new/

[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

martong marked 8 inline comments as done. martong added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:124 + case DeclarationName::CXXConversionFunctionName: +return true; + a_sidorin wrote: > Should we check the equivalence of

[PATCH] D64494: [analyzer]Add user docs rst

martong created this revision. martong added a reviewer: dkrupp. Herald added subscribers: cfe-commits, Charusso, gamesh411, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a project: clang. Add user documentation

[PATCH] D64494: [analyzer]Add user docs rst

martong added a comment. In D64494#1578373 , @dkrupp wrote: > I guess this is a placeholder for the subpages of "User Manual" @ > https://clang-analyzer.llvm.org, which will be ported in follow-up patches. Yes. Repository: rG LLVM Github Monorepo

[PATCH] D64494: [analyzer]Add user docs rst

This revision was automatically updated to reflect the committed changes. Closed by commit rL365639: [analyzer]Add user docs rst (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D64494: [analyzer]Add user docs rst

martong added a comment. > In any case, LGTM, if this really is a beginning of porting the already > existing documentation to sphinx. This is really the beginning of porting the existing docs to sphinx which is supposedly going to be done by @dkrupp. And I am planning to add new user manual

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

martong added a comment. > This patch can be treated as part of a bigger change that is needed to > improve macro expansion handliong at plist generation. @balazske Could you please prepare the upcoming patch and put that on the Stack. I think that may ease the work of the reviewers if they

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

martong added a comment. @shafik @jingham This is a polite Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61333/new/ https://reviews.llvm.org/D61333 ___ cfe-commits mailing list

[PATCH] D64073: [ASTImporter] Fix import of lambda in function param

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. The current import implementation fails to import

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

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 project: clang. The structural equivalence check reported false eq between lambda classes with

[PATCH] D62960: Add SVE opaque built-in types

martong added a comment. The ASTImporter and the test for it looks good to me, thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62960/new/ https://reviews.llvm.org/D62960 ___ cfe-commits mailing list

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added inline comments. Comment at: clang/include/clang/AST/ASTImporter.h:331 +/// +/// \return the equivalent APValue in the "from" Constext or the import +/// error. martong wrote: > typo: `Constext` -> `Context` > Also, we return with the

[PATCH] D63920: [CTU] Add support for virtual functions

martong updated this revision to Diff 208002. martong added a comment. - Remove ipa mode from the test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63920/new/ https://reviews.llvm.org/D63920 Files: clang/lib/StaticAnalyzer/Core/CallEvent.cpp

[PATCH] D63920: [CTU] Add support for virtual functions

martong added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63920/new/ https://reviews.llvm.org/D63920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL364752: [ASTImporter] Propagate error from ImportDeclContext (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

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

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 about the error handling only, so I'd

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

martong updated this revision to Diff 207288. martong marked an inline comment as done. martong added a comment. - Move ImportPathTy's funcitons in-class, add class comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62375/new/

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

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

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

martong marked 3 inline comments as done. martong added a comment. > Thank you for the explanation. I got the idea of this patch anyway, but it > will be definitely useful for anyone digging into the code. Should we make it > a comment for ImportPathTy? Ok, I have added the explanation to

[PATCH] D63640: [clang] Improve Serialization/Imporing of APValues

martong added a comment. Comment at: clang/include/clang/AST/ASTImporter.h:331 +/// +/// \return the equivalent APValue in the "from" Constext or the import +/// error. typo: `Constext` -> `Context` Also, we return with the APValue in the "to"

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

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 about the error handling only, so I'd

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL364771: [ASTImporter] Mark erroneous nodes in from ctx (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

This revision was automatically updated to reflect the committed changes. Closed by commit rL364785: [ASTImporter] Mark erroneous nodes in shared st (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 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 about the error handling only, so I'd

[PATCH] D62329: [ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

martong updated this revision to Diff 207354. martong added a comment. - Handle ImplicitCastExpr. In Clang7 in the last two test cases we did not have any ImplicitCastExpr in the AST. With never Clang we have, so we must handle the cast expr too. Repository: rG LLVM Github Monorepo CHANGES

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

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

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

martong added a comment. Jenkins had one unrelated failure at http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/29812/ Test Result (1 failure / +1) LLDB.Reproducer.TestGDBRemoteRepro.test After that the next build is green:

[PATCH] D64628: [CrossTU] Test change only: improve ctu-main.c

martong added a comment. In D64628#1584969 , @balazske wrote: > The mentioned commit has a test that reproduces the same problem, so this > revision is not needed (abandon it?). I think we should have this change, because it tests also if we can import

[PATCH] D64477: [ASTImporter] Using Lang_CXX14 in ASTImporterVisibilityTest.

martong accepted this revision. martong added a comment. Does it really matter if it is CXX11 or CXX14, in the child patch we use a CXX11 using directive. Anyway, CXX14 is more future proof. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64477/new/

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

martong resigned from this revision. martong added a comment. The changes in `ASTImporter.cpp` looks good to me! And I have no competence about the rest of the change, thus I resign as a reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56571/new/ https://reviews.llvm.org/D56571

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

martong added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3126 +auto Imp = +importSeq(FromConstructor->getExplicitSpecifier().getPointer()); +if (!Imp) Why is it needed to import the explicit specifier here again? You

[PATCH] D43357: [Concepts] Function trailing requires clauses

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. From the `ASTImporter` point of view it looks good, thanks for the changes! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43357/new/

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. From the `ASTImporter` point of view it looks good to me, thanks for the changes! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41910/new/

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

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

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

martong updated this revision to Diff 194527. martong marked an inline comment as done. martong added a comment. - Put back the call to GetOriginalDecl Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692/new/ https://reviews.llvm.org/D59692 Files:

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

martong added a comment. In D60934#1475908 , @Tyker wrote: > Fixed issues form feedback by @martong Thank you for addressing the comments. `ASTImporter.cpp` and `ASTStructuralEquivalence.cpp` looks good to me now. However, about the equivalence check,

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

martong requested changes to this revision. martong added a reviewer: balazske. martong added inline comments. This revision now requires changes to proceed. Comment at: lib/AST/ASTImporter.cpp:5026 +ClassTemplate->findPartialSpecialization(TemplateArgs, +

[PATCH] D44352: [Concepts] Constrained template parameters

martong requested changes to this revision. martong added inline comments. This revision now requires changes to proceed. Herald added a subscriber: rnkovacs. Comment at: lib/AST/ASTImporter.cpp:3708 + if (Expr *CE = D->getConstraintExpression()) +

[PATCH] D43357: [Concepts] Function trailing requires clauses

martong requested changes to this revision. martong added inline comments. This revision now requires changes to proceed. Comment at: lib/AST/ASTImporter.cpp:3053 +D->isInlineSpecified(), D->isImplicit(), D->isConstexpr(), +InheritedConstructor(),

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

martong added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:965 auto *Conversion2 = cast(Method2); -if (Conversion1->isExplicit() != Conversion2->isExplicit()) +if (!Conversion1->isEquivalentExplicit(Conversion2->getExplicitSpecifier()))

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

martong added inline comments. Comment at: include/clang/AST/ASTImporter.h:234 +/// The ASTUnit that this importer belongs to, if any. +ASTUnit *Unit; + `Unit->getASTContext` must be equal to `FromContext`, right? Then `FromUnit` would be a better

[PATCH] D64753: [CrossTU][NFCI] Refactor loadExternalAST function

martong added a comment. Thank you Endre, this patch is a great initiative. However, I think we can do better encapsulation then just the reorganization of the functions: We could encapsulate into a nested class `NameASTUnitMap` and the functions which operate on this

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

martong added inline comments. Comment at: include/clang/AST/ASTImporter.h:317 +std::shared_ptr SharedState = nullptr, +ASTUnit *Unit = nullptr); balazske wrote: > balazske wrote: > > martong wrote: > > > What if we provided an

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

martong added inline comments. Comment at: include/clang/AST/ASTImporter.h:317 +std::shared_ptr SharedState = nullptr, +ASTUnit *Unit = nullptr); martong wrote: > balazske wrote: > > balazske wrote: > > > martong wrote: > > > >

[PATCH] D64554: [CrossTU] Add a function to retrieve original source location.

martong added inline comments. Comment at: include/clang/AST/ASTImporter.h:317 +std::shared_ptr SharedState = nullptr, +ASTUnit *Unit = nullptr); What if we provided an additional constructor where we take over the ASTUnits

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

martong added a comment. In D61333#1583173 , @teemperor wrote: > @martong Sorry for the delay, feel free to ping me in the future about these > patches. I'll review them ASAP now that I'm back in office, so these delay's > hopefully won't happen again.

[PATCH] D64477: [ASTImporter] Using Lang_CXX14 in ASTImporterVisibilityTest.

martong added a comment. In D64477#1585218 , @balazske wrote: > In D64477#1585092 , @martong wrote: > > > Does it really matter if it is CXX11 or CXX14, in the child patch we use a > > CXX11 using directive.

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