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

[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: clang/incl

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

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

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

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

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

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

martong updated this revision to Diff 209832. martong added a comment. - Rebase to master 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 clang/unittests/AST/ASTImpo

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

[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 (`getCachedASTUnitForName`,

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

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

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

[PATCH] D64801: [analyzer] Add CTU user docs

martong created this revision. martong added reviewers: dkrupp, a_sidorin, Szelethus, NoQ. Herald added subscribers: cfe-commits, Charusso, gamesh411, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity. Herald added a project: clang. Repository:

[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/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py:57 +# This expr command imports __sFILE with definition +# (FILE is a typedef to __sFILE.) +self.expect(

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

martong updated this revision to Diff 210281. martong marked 5 inline comments as done. martong added a comment. - Applied clang-format on lldb parts (this changed two lines) - Added a comment for predicate - Merged the test into TestCModules.py Repository: rG LLVM Github Monorepo CHANGES SIN

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL366325: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to comm

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

martong added a comment. Thank you guys for the review! 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] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

martong added a comment. Jenkins looks okay: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31157/ .The build is red but the the previous run has the very same failing test case: expression_command/weak_symbols/TestWeakSymbols.py Repository: rL LLVM CHANGES SINCE LAST ACTION htt

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

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/ . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64075/

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL366332: [ASTImporter] Fix structural eq of lambdas (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.l

[PATCH] D64801: [analyzer] Add CTU user docs

martong updated this revision to Diff 210345. martong marked 3 inline comments as done. martong added a comment. - Address dkrupp's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64801/new/ https://reviews.llvm.org/D64801 Files: clang/do

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

martong added a comment. Jenkins looks okay: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31160/ .The build is red but the the previous run has the very same failing test case: expression_command/weak_symbols/TestWeakSymbols.py Repository: rL LLVM CHANGES SINCE LAST ACTION htt

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

martong added inline comments. Comment at: include/clang/AST/ASTImporter.h:288 +ASTContext &FromContext, FileManager &FromFileManager, +ASTUnit *FromUnit, bool MinimalImport, +std::shared_ptr SharedState); Why do we

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM. Comment at: include/clang/AST/ASTImporter.h:288 +ASTContext &FromContext, FileManager &FromFileManager, +ASTUnit *FromUnit, bool Minim

[PATCH] D64801: [analyzer] Add CTU user docs

martong updated this revision to Diff 210552. martong added a comment. - Add notes about macro expansion crash with CTU Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64801/new/ https://reviews.llvm.org/D64801 Files: clang/docs/analyzer/user-docs

[PATCH] D64801: [analyzer] Add CTU user docs

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

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

martong updated this revision to Diff 210581. martong added a comment. - Add test case ClassTemplSpecWithInequivalentShadowedTemplArg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64241/new/ https://reviews.llvm.org/D64241 Files: clang/lib/AST/A

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

martong added a comment. In D64241#1584972 , @a_sidorin wrote: > Hi Gabor, > This looks fine, but could you please add a test showing how decl shadowing > is handled? I.e. if we have Arg in one TU and both Arg and N::Arg in another > TU. Alexei, Than

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

martong updated this revision to Diff 210813. martong added a comment. - Rebase to master - Some refactor is done mostly because since D63603 ([ASTImporter] Propagate error from ImportDeclContext) we may not imported successfully all decls of a DC. - Made the c

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

martong updated this revision to Diff 210861. martong added a comment. - Further simplify by removing the last for loop Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44100/new/ https://reviews.llvm.org/D44100 Files: clang/lib/AST/ASTImporter.cpp

[PATCH] D65042: [Concept] Placeholder constraints and abbreviated templates

martong added a comment. `ASTImporter.cpp` and `ASTStructuralEquivalence.cpp` looks good to me! Comment at: lib/AST/ASTImporter.cpp:1286 - return Importer.getToContext().getAutoType(*ToDeducedTypeOrErr, - T->getKeyword(), -

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65064/new/ https://reviews.llvm.org/D65064

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

martong added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:476 + NewImporter->setFileIDImportHandler( + [this, Unit](FileID ToID, FileID FromID, ASTImporter &Importer) { +assert(ImportedFileIDs.find(ToID) == ImportedFileIDs.end() && --

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

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

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

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

[PATCH] D64638: [CrossTU] Fix plist macro expansion if macro in other file.

martong added a subscriber: NoQ. martong added a comment. In D64638#1593382 , @ilya-biryukov wrote: > `StaticAnalyzer/Core` does not depend on `clangFrontend` now, you can see > this by looking at `lib/StaticAnalyzer/Core/CMakeLists.txt`: > > add_clang

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

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

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

martong updated this revision to Diff 211312. martong added a comment. - Rebase to master Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692/new/ https://reviews.llvm.org/D59692 Files: clang/include/clang/AST/ASTImporter.h clang/lib/AST/ASTIm

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

martong updated this revision to Diff 211314. martong marked 5 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/D44100/new/ https://reviews.llvm.org/D44100 Files: clang/li

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

martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:5244 +} + + a_sidorin wrote: > A redundant newline? Yes, I deleted that. And moved the test case closer the the last `ImportDecl` test case. Repository: rG LLVM Github Monorep

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

martong added a comment. I have a feeling that LoadPass and LoagGuard could be (should be) merged together, other than that we are getting close. Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:185 + + /// Cached access to ASTUnit mapping information is impleme

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL366997: [ASTImporter] Reorder fields after structure import is finished (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit

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

martong added a comment. Jenkins is green: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31743/ The previous build caught up the change, but http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31742/ was stopped, perhaps a test got stucked. Repository: rL LLVM CHANGES SINCE

[PATCH] D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.

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

[PATCH] D65445: [CrossTU] Handle case when no USR could be generated during Decl search.

martong added a comment. Thank you for this patch Balazs! Could you please elaborate in which cases we cannot generate the USR? Does it happen only if we have an anonymous union inside a function? Are there no other cases? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D65445: [CrossTU] Handle case when no USR could be generated during Decl search.

martong added a comment. In D65445#1607803 , @balazske wrote: > It looks like that the problem can happen when the anonymous union is in any > `DeclContext` and for CTU import the import of a variable is requested and > that variable is in a related `Dec

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

martong added inline comments. Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:489 + if (DisplayCTUProgress) { +if (llvm::Expected FileName = +ASTStorage.getFileForFunction(LookupName, CrossTUDir, IndexName)) ` LoadOperation` should not be

[PATCH] D65445: [CrossTU] Handle case when no USR could be generated during Decl search.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Please proceed and commit! :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65445/new/ https://reviews.llvm.org/D65445 __

[PATCH] D65445: [CrossTU] Handle case when no USR could be generated during Decl search.

martong added a comment. In D65445#1608277 , @balazske wrote: > Probably this is a problem case too but only if the `f` or `i` has an > initializer expression and really no USR is generated for `f` or `i`. But > what I have found is that there is a `VarD

[PATCH] D65573: Add User docs for ASTImporter

martong created this revision. martong added reviewers: a_sidorin, shafik, gamesh411, balazske. Herald added subscribers: cfe-commits, Szelethus, arphaman, dkrupp, rnkovacs. Herald added a reviewer: a.sidorin. Herald added a project: clang. This document includes the description of the ASTImporter

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

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Please do the commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64753/new/ https://reviews.llvm.org/D64753 __

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM, though I have some comments. Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:135 + +struct DefParmContext { + static const int I; Perhaps we co

[PATCH] D65573: Add User docs for ASTImporter

martong marked 23 inline comments as done. martong added a comment. Thanks for the review! Comment at: clang/docs/LibASTImporter.rst:19 +``ASTContext`` holds long-lived AST nodes (such as types and decls) that can be referred to throughout the semantic analysis of a file. +The

[PATCH] D65573: Add User docs for ASTImporter

martong updated this revision to Diff 213046. martong marked 5 inline comments as done. martong added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65573/new/ https://reviews.llvm.org/D65573 Files: clang/docs/LibAST

[PATCH] D65573: Add User docs for ASTImporter

martong updated this revision to Diff 213332. martong added a comment. - Add description for `-ast-merge` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65573/new/ https://reviews.llvm.org/D65573 Files: clang/docs/LibASTImporter.rst clang/docs/

[PATCH] D65573: Add User docs for ASTImporter

This revision was automatically updated to reflect the committed changes. Closed by commit rL368009: Add User docs for ASTImporter (authored by martong, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D6557

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

martong accepted this revision. martong added a comment. Still looks good to me, other than some style nits. Comment at: clang/lib/AST/ASTImporter.cpp:3829 +Error ASTNodeImporter::ImportDefaultArgOfParmVarDecl(const ParmVarDecl *FromParam, ParmVarDecl *ToParam) { + ToParam-

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

martong added a comment. Would it be an alternative solution if we imported the function parameters after we created the FunctionDecl? I guess it would be. Maybe that would result in a smaller change, what do you think? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

martong added a comment. LGTM, but I don't exactly see how the first test is related. Could you please explain? Comment at: clang/unittests/AST/ASTImporterTest.cpp:5239 +TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) { + // Test that import of i

[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

martong added inline comments. Comment at: clang/test/Analysis/Inputs/ctu-other.cpp:135 + +struct DefParmContext { + static const int I; balazske wrote: > martong wrote: > > shafik wrote: > > > martong wrote: > > > > Perhaps we could write `Default` instead of `

[PATCH] D66054: [CrossTU] Fix problem with CrossTU AST load limit and progress messages.

martong accepted this revision. martong added a reviewer: gamesh411. martong added a comment. This revision is now accepted and ready to land. LGTM! Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66054/new/ https://reviews.llvm.org/D66054

[PATCH] D66336: [ASTImporter] Add development internals docs

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

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

martong marked an inline comment as done. martong added a comment. In D59692#1632138 , @shafik wrote: > Just wanted to see if you were planning on landing this soon, the fix `Name` > -> `SearchName` is probably an important one since we have seen several

[PATCH] D64464: [CodeGen] Emit destructor calls for non-trivial C structs

martong added inline comments. Comment at: test/Import/objc-arc/Inputs/cleanup-objects.m:6 +id getObj(int c, id a) { + // Commenting out the following line because AST importer crashes when trying + // to import a BlockExpr. Perhaps then this patch depends on a

[PATCH] D65573: Add User docs for ASTImporter

martong marked 3 inline comments as done. martong added a comment. In D65573#1625881 , @a_sidorin wrote: > That's incredible. Thank you! Thanks Alexei for the review! I commited a fix for the typos. Comment at: cfe/trunk/docs/LibASTIm

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

martong updated this revision to Diff 215579. martong added a comment. - Name -> SearchName - Add tests for conflicting declarations Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59692/new/ https://reviews.llvm.org/D59692 Files: clang/include/cl

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

martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:2392 +struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {}; + shafik wrote: > What about tests for name conflicts for:

[PATCH] D66348: [ASTImporter] Do not look up lambda classes

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. Consider this code: void f() { auto L0 = [](

[PATCH] D64078: [ASTImporter] Fix structural ineq of lambdas with different sloc

martong added a comment. In D64078#1572675 , @a_sidorin wrote: > Hi Gabor, > it is a nice design question if source locations can participate in > structural match or not. The comparison tells us that the same code written > in different files is not st

[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: clang/include/clang/AST/AS

[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: clang/lib/AST/ASTImporter.

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

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

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

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

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

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

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

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

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

[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] 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`. > > ht

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

[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: clang/lib/AST

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

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

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

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

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

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

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

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

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

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