Page MenuHomePhabricator

a_sidorin (Aleksei Sidorin)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 24 2018, 6:54 AM (25 w, 3 d)

Recent Activity

Sun, Dec 16

a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Gabor, thank you a lot!
I have to say that even having your patch, I still don't understand why the patch fixes the problem (and what the problem is). As I see, the patch handles the cases where some fields or friends have DeclContext different from ToRD. Could you please provide examples so I can add them to the test suite or, at least, just investigate?

Sun, Dec 16, 3:51 PM

Sun, Dec 9

a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Gabor,
Thank you for the investigation! I'll try to take a look.

Sun, Dec 9, 8:59 AM
a_sidorin added a comment to D55387: [analyzer] MoveChecker Pt.7: NFC: Misc refactoring..

Hi Artem,
The overall idea is good but I have some remarks inline.

Sun, Dec 9, 8:58 AM

Sat, Dec 8

a_sidorin accepted D55307: [analyzer] MoveChecker Pt.6: Suppress the warning for the few move-safe STL classes..

I think the change is fine, just a minor stylish remark.

Sat, Dec 8, 11:50 PM

Thu, Dec 6

a_sidorin accepted D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies..

LG!

Thu, Dec 6, 2:30 PM
a_sidorin accepted D55129: [CTU] Eliminate race condition in CTU lit tests.
Thu, Dec 6, 1:28 PM

Wed, Dec 5

a_sidorin accepted D55133: [CTU] Add statistics.

LGTM with a nit.

Wed, Dec 5, 1:53 PM
a_sidorin requested changes to D55280: [CTU] Make loadExternalAST return with non nullptr on success.

Hi Gabor,
There is a code in getExternalAST:

std::unique_ptr<ASTUnit> LoadedUnit(ASTUnit::LoadFromASTFile(
    ASTFileName, CI.getPCHContainerOperations()->getRawReader(),
    ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts()));
Unit = LoadedUnit.get();
FileASTUnitMap[ASTFileName] = std::move(LoadedUnit);

And ASTUnit::LoadFromASTFile()can return nullptr. Actually, there is a problem in loadExternalAST() - it ignores this fact.

Wed, Dec 5, 1:24 PM

Tue, Dec 4

a_sidorin added a comment to D55289: [analyzer] MoveChecker Pt.5: Improve invalidation policies..

Hi Artem,
The change looks fine, just some nits.

Tue, Dec 4, 10:40 PM

Mon, Dec 3

a_sidorin accepted D55134: [CTU] Add triple/lang mismatch handling.

LGTM, thanks!

Mon, Dec 3, 12:30 PM

Sat, Dec 1

a_sidorin added a comment to D55134: [CTU] Add triple/lang mismatch handling.

Hi Gabor,
Please find my comments inline.

Sat, Dec 1, 1:47 PM
a_sidorin added a comment to D55135: [CTU][Analyzer]Add DisplayCTUProgress analyzer switch.

Hi Gabor,
In addition to Umann remarks, there is a small comment inline.

Sat, Dec 1, 1:20 PM
a_sidorin added a comment to D55131: [CTU] Add more lit tests and better error handling.

Hi Gabor,
Please find my comments inline.

Sat, Dec 1, 1:15 PM
a_sidorin added a comment to D55133: [CTU] Add statistics.

Hi Gabor,
Sorry, but I don't understand the meaning of some options. Could you please explain what are NumNoUnit and NumNotInOtherTU and what is the difference between them?

Sat, Dec 1, 12:41 PM
a_sidorin accepted D55132: [CTU] Add asserts to protect invariants.

More assertions are always good.

Sat, Dec 1, 12:32 PM
a_sidorin added a comment to D55129: [CTU] Eliminate race condition in CTU lit tests.

Hi Gabor,
Yes, the %T variable is obsolete, but it looks like the Guide recommendations are not entirely fulfilled.

Sat, Dec 1, 12:30 PM
a_sidorin accepted D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull.

LGTM with a nit: the call to GetAlreadyImportedOrNull is not removed but moved into ImportDeclParts.

Sat, Dec 1, 12:23 PM
a_sidorin accepted D53699: [ASTImporter] Fix inequality of functions with different attributes.

LGTM, thanks!

Sat, Dec 1, 12:11 PM
a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Gabor,
Thank you for digging into this! Unfortunately, I don't have any MacOS device and I cannot check my code for the compatibility.

Sat, Dec 1, 11:54 AM
a_sidorin accepted D53708: [ASTImporter] Add importer specific lookup.

Hi Gabor,
I think the code looks good. Thank you!

Sat, Dec 1, 11:44 AM

Wed, Nov 28

a_sidorin added a comment to D53708: [ASTImporter] Add importer specific lookup.

Hi Gabor,
Here are some new comments.

Wed, Nov 28, 11:44 PM
a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Adrian,
I've already done check-all (this includes LLDB) with this patch. As you can see in this thread, I didn't manage to reproduce some issues on Linux. Luckily, one test still failed on Linux so I could find the problem. However , it will be good to ensure that this is the same problem that caused failures on MacOS and the commit will be safe. That's why I asked for an additional check.

Wed, Nov 28, 10:07 PM

Mon, Nov 26

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

LGTM, thanks!

Mon, Nov 26, 1:00 PM

Sun, Nov 25

a_sidorin accepted D53693: [ASTImporter] Typedef import brings in the complete type.

Thanks!

Sun, Nov 25, 11:21 AM
a_sidorin added a comment to D53755: [ASTImporter] Remove import of definition from GetAlreadyImportedOrNull.

More constants are always welcome.

Sun, Nov 25, 11:10 AM
a_sidorin edited reviewers for D53751: [ASTImporter] Added Import functions for transition to new API., added: a_sidorin; removed: a.sidorin.
Sun, Nov 25, 11:07 AM
a_sidorin accepted D54863: [ASTImporter] Set MustBuildLookupTable on PrimaryContext.

Hi Gabor,
This change looks fine to me but please wait for LLDB reviewer's decision.

Sun, Nov 25, 11:06 AM
a_sidorin updated subscribers of D53655: [ASTImporter] Fix redecl chain of classes and class templates.

Please note that changes in AST lib will require AST code owners' approval. @rsmith, @aaron.ballman could you please take a look?

Sun, Nov 25, 11:03 AM
a_sidorin updated subscribers of D53751: [ASTImporter] Added Import functions for transition to new API..

Hi Balazs,
The changes look mostly fine to me. I think they can be accepted after some minor stylish fixes.

Sun, Nov 25, 10:54 AM
a_sidorin added a comment to D53655: [ASTImporter] Fix redecl chain of classes and class templates.

Hi Gabor,

Sun, Nov 25, 10:10 AM
a_sidorin accepted D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter..

LGTM. Thank you for addressing my questions!

Sun, Nov 25, 8:37 AM
a_sidorin updated subscribers of D53708: [ASTImporter] Add importer specific lookup.

Hi Gabor,

Sun, Nov 25, 8:35 AM

Nov 18 2018

a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Davide,

Nov 18 2018, 10:11 PM
a_sidorin updated the diff for D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi @davide and @shafik,
Could you please check the updated version of the patch?

Nov 18 2018, 2:37 PM
a_sidorin reopened D44100: [ASTImporter] Reorder fields after structure import is finished.
Nov 18 2018, 2:34 PM

Nov 14 2018

a_sidorin accepted D53702: [ASTImporter] Set redecl chain of functions before any other import.

Hi Gabor,
The change looks fine. Thanks!

Nov 14 2018, 10:35 PM
Herald added a reviewer for D53818: [ASTImporter] Changed use of Import to Import_New in ASTImporter.: shafik.

Hi Balasz,

Nov 14 2018, 1:46 PM

Oct 30 2018

a_sidorin added a comment to D53699: [ASTImporter] Fix inequality of functions with different attributes.

Hi Gabor,
Thank you for the patch. The reason for this change looks clear. However, I don't think omitting this comparison completely is what we want here. Instead, we can do a dance similar to ASTContext::mergeFunctionTypes() where all attributes but NoReturn are compared. What do you think?

Oct 30 2018, 3:37 PM
a_sidorin abandoned D50672: [ASTImporter] Change the return result of Decl import to Optional .

Another version has been committed (D51633).

Oct 30 2018, 2:42 PM
a_sidorin accepted D53697: [ASTImporter][Structural Eq] Check for isBeingDefined.

Thank you for the explanation!

Oct 30 2018, 2:39 PM
a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Oops. Thank you Davide!

Oct 30 2018, 2:10 PM

Oct 27 2018

a_sidorin accepted D53704: [ASTImporter] Import overrides before importing the rest of the chain.

Hello Gabor,

Oct 27 2018, 4:18 PM

Oct 26 2018

a_sidorin added a comment to D53697: [ASTImporter][Structural Eq] Check for isBeingDefined.

Hi Gabor,
I wonder if it is possible to get into situation where non-equivalent decls are marked equivalent with this patch? If yes, we can create a mapping between decls being imported and original decls as an alternative solution. However, I cannot find any counterexample.

Oct 26 2018, 3:30 PM
a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Hi Gabor,
I have tried to rebase it last week but there were some test failures. I hope to fix them this weekend and finally commit the patch.

Oct 26 2018, 8:58 AM
a_sidorin added a comment to D53693: [ASTImporter] Typedef import brings in the complete type.

Hi Gabor,
Thank you for the patch. I looks reasonable but I have some questions.

Oct 26 2018, 8:46 AM

Oct 18 2018

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

Hi Balasz,
I cannot find any problems with the latest changes. I suggest you to commit the patch to make further reviews easier.

Oct 18 2018, 2:50 PM

Oct 14 2018

a_sidorin added a comment to D51633: [ASTImporter] Added error handling for AST import..

Hi Balázs,

Oct 14 2018, 3:48 PM

Sep 30 2018

a_sidorin added a comment to D51633: [ASTImporter] Added error handling for AST import..

Hi Balasz,
It's going to take some time to review the whole patch.

Sep 30 2018, 3:18 PM

Sep 16 2018

a_sidorin added a comment to D51633: [ASTImporter] Added error handling for AST import..

Hi Gabor,

Sep 16 2018, 3:52 PM

Sep 15 2018

a_sidorin accepted D51597: [ASTImporter] Fix import of VarDecl init.
Sep 15 2018, 3:10 PM

Sep 9 2018

a_sidorin added a comment to D51597: [ASTImporter] Fix import of VarDecl init.

Hi Gabor,
The change looks mostly fine but the difference with ASTReader approach disturbs me a bit.

Sep 9 2018, 2:06 PM

Sep 2 2018

a_sidorin accepted D51533: [ASTImporter] Merge ExprBits.

Looks good, thanks!

Sep 2 2018, 1:51 PM

Aug 23 2018

a_sidorin accepted D51178: [ASTImporter] Add test for importing anonymous namespaces..
Aug 23 2018, 11:30 PM
a_sidorin accepted D51142: [ASTImporter] Add test for PackExpansionExpr.
Aug 23 2018, 11:17 PM

Aug 22 2018

a_sidorin accepted D51056: [ASTImporter] Add test for SwitchStmt.

Thank you for working on this!

Aug 22 2018, 3:32 PM
a_sidorin accepted D51110: [ASTImporter] Remove duplicated and dead CXXNamedCastExpr handling code..
Aug 22 2018, 3:30 PM
a_sidorin accepted D51115: [ASTImporter] Actually test ArrayInitLoopExpr in the array-init-loop-expr test..

Wow, I totally overlooked this. Thank you!

Aug 22 2018, 3:26 PM
a_sidorin accepted D51121: [ASTImporter] Add test for ObjCAtTryStmt/ObjCAtCatchStmt/ObjCAtThrowStmt.
Aug 22 2018, 3:24 PM
a_sidorin accepted D51123: [ASTImporter] Add test for ObjCAutoreleasePoolStmt.
Aug 22 2018, 3:19 PM
a_sidorin accepted D51059: [ASTImporter] Add test for ObjCTypeParamDecl.
Aug 22 2018, 3:19 PM

Aug 21 2018

a_sidorin accepted D50451: [ASTImporter] Fix import of class templates partial specialization.

Thank you!

Aug 21 2018, 11:33 PM

Aug 20 2018

a_sidorin requested changes to D50662: Add dump() method for SourceRange.

Hello Stephen,
These methods will be really useful.

Aug 20 2018, 3:02 PM
a_sidorin accepted D50978: [ASTImporter] Add test for C++'s try/catch statements..
Aug 20 2018, 1:45 PM

Aug 19 2018

a_sidorin accepted D49798: [ASTImporter] Adding some friend function related unittests..
Aug 19 2018, 8:15 AM
a_sidorin accepted D50928: [ASTImporter] Test for importing condition variable from a ForStmt.
Aug 19 2018, 8:09 AM
a_sidorin added inline comments to D50451: [ASTImporter] Fix import of class templates partial specialization.
Aug 19 2018, 8:08 AM
a_sidorin accepted D50932: [ASTImporter] Add test for C++ casts and fix broken const_cast importing..

Thank you!

Aug 19 2018, 6:44 AM
a_sidorin accepted D50737: [ASTImporter] Add test for CXXNoexceptExpr.

Thanks!

Aug 19 2018, 6:41 AM

Aug 15 2018

a_sidorin accepted D50812: [ASTImporter] Add test for ForStmt and ContinueStmt.
Aug 15 2018, 5:14 PM
a_sidorin accepted D50735: [ASTImporter] Add test for CXXScalarValueInit.
Aug 15 2018, 5:09 PM
a_sidorin accepted D50732: [ASTImporter] Add test for CXXDefaultInitExpr.

As a side note: It seems this test case actually reveals that we don't import the body of Foo's destructor?

This is strange. If you manage to find the reason, please notify us!

Aug 15 2018, 5:08 PM
a_sidorin added a comment to D49798: [ASTImporter] Adding some friend function related unittests..

Hi Balasz,

Aug 15 2018, 5:03 PM
a_sidorin added a comment to D50672: [ASTImporter] Change the return result of Decl import to Optional .

Hello Gabor and Balazs,

Aug 15 2018, 4:56 PM
a_sidorin added inline comments to D44100: [ASTImporter] Reorder fields after structure import is finished.
Aug 15 2018, 3:51 PM
a_sidorin accepted D50733: [ASTImporter] Add test for ArrayInitLoopExpr.
Aug 15 2018, 3:50 PM
a_sidorin accepted D50731: [ASTImporter] Add test for ExprWithCleanups.

Tests are always welcome. Thanks!

Aug 15 2018, 3:45 PM

Aug 13 2018

a_sidorin added inline comments to D44100: [ASTImporter] Reorder fields after structure import is finished.
Aug 13 2018, 3:59 PM
a_sidorin updated the diff for D44100: [ASTImporter] Reorder fields after structure import is finished.

All declarations are reordered now, not only fields. Also some review comments were addressed.

Aug 13 2018, 3:57 PM
a_sidorin commandeered D44100: [ASTImporter] Reorder fields after structure import is finished.
Aug 13 2018, 3:55 PM
a_sidorin created D50672: [ASTImporter] Change the return result of Decl import to Optional .
Aug 13 2018, 3:52 PM

Aug 11 2018

a_sidorin accepted D50550: [ASTImporter] Added test case for opaque enums.

LGTM!

Aug 11 2018, 1:32 PM
a_sidorin accepted D50552: [ASTImporter] Added test case for CXXConversionDecl importing.

Thanks!

Aug 11 2018, 1:28 PM
a_sidorin accepted D50516: [ASTImporter] Improved import of friend templates..
Aug 11 2018, 1:26 PM
a_sidorin added a comment to D49798: [ASTImporter] Adding some friend function related unittests..

Hi Balazs,

Aug 11 2018, 10:46 AM
a_sidorin added a comment to D46940: [ASTImporter] make sure that ACtx::getParents still works.

Hello Richard,

Aug 11 2018, 8:06 AM
a_sidorin added a comment to D50451: [ASTImporter] Fix import of class templates partial specialization.

Hi Gabor,

Aug 11 2018, 8:01 AM

Aug 8 2018

a_sidorin accepted D50444: [ASTImporter] Fix structural inequivalency of forward EnumDecl.

Yes, this seems to be correct. Thanks!

Aug 8 2018, 3:39 PM
a_sidorin accepted D50428: [ASTImporter] Add support for importing imaginary literals.

LGTM! Just a stylish nit.

Aug 8 2018, 2:59 PM

Aug 3 2018

a_sidorin added inline comments to D49223: [AST] Check described template at structural equivalence check..
Aug 3 2018, 3:17 PM · Restricted Project
a_sidorin added a comment to D49796: [ASTImporter] Load external Decls when getting field index..

Hi Balázs,
The approach is OK but I have some minor comments inline.

Aug 3 2018, 2:06 PM

Aug 1 2018

a_sidorin accepted D49792: [ASTmporter] SourceRange-free function parameter checking for declarations.

LGTM. Thank you!

Aug 1 2018, 7:41 AM
a_sidorin added inline comments to D49223: [AST] Check described template at structural equivalence check..
Aug 1 2018, 7:28 AM · Restricted Project

Jul 16 2018

a_sidorin added inline comments to D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr..
Jul 16 2018, 3:55 PM
a_sidorin accepted D49235: [ASTImporter] Import described template (if any) of function..

LGTM.

Jul 16 2018, 3:54 PM
a_sidorin accepted D49296: [ASTImporter] Fix import of unnamed structs.

Thank you Gabor!

Jul 16 2018, 3:51 PM
a_sidorin accepted D49300: [ASTImporter] Fix poisonous structural equivalence cache.

Hi Gabor,

Jul 16 2018, 3:29 PM

Jul 14 2018

a_sidorin added a comment to D49296: [ASTImporter] Fix import of unnamed structs.

Hi Gabor,
The change is OK but I have some questions regarding tests.

Jul 14 2018, 11:18 PM
a_sidorin added a comment to D49300: [ASTImporter] Fix poisonous structural equivalence cache.

Hi Gabor,
Could you provide some tests for the issue?

Jul 14 2018, 11:07 PM
a_sidorin added a comment to D49293: [ASTImporter] Add support for import of CXXInheritedCtorInitExpr..

Adding new nodes is always welcome.

Jul 14 2018, 10:56 PM
a_sidorin accepted D49245: [ASTImporter] Import implicit methods of existing class..

LGTM. Just some stylish nits.
To resolve this issue, I used Sema::DeclareImplicit... methods. But I like this approach much more because it doesn't allows to forget different kinds of implicit methods and doesn't require passing Sema into ASTImporter.

Jul 14 2018, 10:42 PM