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 (38 w, 3 d)

Recent Activity

Mon, Mar 11

a_sidorin added a comment to D58897: [ASTImporter] Make ODR error handling configurable.

Hi Gabor,
This patch LGTM mostly, but there is a comment inline.

Mon, Mar 11, 12:34 AM · Restricted Project

Sun, Mar 10

a_sidorin accepted D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec.

Thanks!

Sun, Mar 10, 10:51 AM · Restricted Project, Restricted Project
a_sidorin accepted D55358: [ASTImporter] Fix import of NestedNameSpecifierLoc..

Looks good!

Sun, Mar 10, 8:37 AM · Restricted Project, Restricted Project

Sun, Mar 3

a_sidorin accepted D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec.

Hi Gabor,
Thanks for the patch! It looks good to me except some stylish nits.

Sun, Mar 3, 11:00 AM · Restricted Project, Restricted Project
a_sidorin added a comment to D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec.

Hi Gabor,
The patch looks almost good bu I have some comments inline.

Sun, Mar 3, 10:56 AM · Restricted Project, Restricted Project
a_sidorin accepted D58830: [ASTImporter] Import member expr with explicit template args.
Sun, Mar 3, 6:33 AM · Restricted Project, Restricted Project

Tue, Feb 26

a_sidorin accepted D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls.

Thanks for the fixes!

Tue, Feb 26, 2:02 PM · Restricted Project
a_sidorin added a comment to D57590: [ASTImporter] Improve import of FileID..

Hi Gabor,
Sorry, missed the patch. It looks mostly good, but do we have any test for this?

Tue, Feb 26, 1:59 PM · Restricted Project

Sun, Feb 24

a_sidorin added a comment to D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls.

Hi Gabor,
The patch looks OK overall but I have some comments inline.

Sun, Feb 24, 3:32 PM · Restricted Project
a_sidorin accepted D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate.

Hi Gabor,
I don't see any problems with the patch. Thanks! I think it will be good to get Shafik's approval as well.

Sun, Feb 24, 3:21 PM · Restricted Project, Restricted Project

Sat, Feb 23

a_sidorin accepted D58292: Add support for importing ChooseExpr AST nodes..

Hi Tom,
Thanks for the fixes! The patch looks good to me now. I have only a small nit inline.

Sat, Feb 23, 1:02 AM · Restricted Project, Restricted Project

Thu, Feb 21

a_sidorin added inline comments to D58292: Add support for importing ChooseExpr AST nodes..
Thu, Feb 21, 1:46 PM · Restricted Project, Restricted Project

Wed, Feb 20

a_sidorin added a comment to D58292: Add support for importing ChooseExpr AST nodes..

Hi Tom,

Wed, Feb 20, 4:21 PM · Restricted Project, Restricted Project

Feb 16 2019

a_sidorin requested changes to D58292: Add support for importing ChooseExpr AST nodes..

Hi Tom,
The change looks reasonable but the tests need some improvements.

Feb 16 2019, 2:19 AM · Restricted Project, Restricted Project
a_sidorin accepted D57236: [ASTImporter] Unify redecl chain tests as type parameterized tests.

Thanks for the changes! The patch looks completely fine to me now.

Feb 16 2019, 1:55 AM · Restricted Project, Restricted Project
a_sidorin accepted D57910: [ASTImporter] Find previous friend function template.

Hi Gabor,
This patch LGTM with a minor nit.

Feb 16 2019, 1:48 AM · Restricted Project

Feb 11 2019

a_sidorin added a comment to D57236: [ASTImporter] Unify redecl chain tests as type parameterized tests.

Hi Gabor,

Feb 11 2019, 3:58 PM · Restricted Project, Restricted Project

Feb 10 2019

a_sidorin accepted D57901: [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces.
Feb 10 2019, 2:34 AM · Restricted Project, Restricted Project

Feb 7 2019

a_sidorin added a comment to D57906: [CTU] Do not allow different CPP dialects in CTU.

Hi Gabor,
Please find my comments inline.

Feb 7 2019, 3:40 PM · Restricted Project
a_sidorin accepted D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators.

LGTM!

Feb 7 2019, 3:35 PM · Restricted Project
a_sidorin accepted D57902: [AST] Fix structural inequivalence of operators.

Looks good, thanks!

Feb 7 2019, 3:25 PM · Restricted Project

Feb 6 2019

a_sidorin accepted D57322: [ASTImporter] Refactor unittests to be able to parameterize them in a more flexible way.
Feb 6 2019, 10:06 PM · Restricted Project

Jan 27 2019

a_sidorin accepted D57235: [AST] Add structural eq tests for template args.

There are never enough tests :) Thank you!

Jan 27 2019, 5:30 AM

Jan 23 2019

a_sidorin accepted D57062: [analyzer] Re-enable the "System is over constrained" assertion on optimized builds..

Oh, I remember how much pain have debug mode-only assertions caused.

Jan 23 2019, 1:32 PM

Jan 21 2019

a_sidorin accepted D56651: [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl.

LGTM, thank you!
I'm sorry if the change I requested doesn't look good, but I think the correctness is our priority.

Jan 21 2019, 2:47 PM

Jan 18 2019

a_sidorin added a comment to D56651: [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl.

Hi Shafik,
Please find my answers inline.

Jan 18 2019, 2:58 PM

Jan 14 2019

a_sidorin added a comment to D56632: [analyzer] Track region liveness only through base regions..

Hi Artem,
This looks perfect, just some stylish issues.

Jan 14 2019, 1:23 PM

Jan 13 2019

a_sidorin added a comment to D56651: [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl.

Hi Rafael,
The change looks mostly fine but I have some comments inline.

Jan 13 2019, 3:19 PM

Jan 12 2019

a_sidorin added a comment to D56581: [ASTImporter] Set the described template if not set.

Hello Gabor,

Jan 12 2019, 10:23 AM

Dec 23 2018

a_sidorin accepted D55280: [CTU] Make loadExternalAST return with non nullptr on success.

Hi Gabor,
Yes, this looks good. Thanks!

Dec 23 2018, 1:24 AM
a_sidorin added inline comments to D55734: [analyzer] Revise GenericTaintChecker's internal representation.
Dec 23 2018, 1:18 AM
a_sidorin updated subscribers of D55646: [ASTImporter] Make ODR diagnostics warning by default.

Hello Endre.
I agree that it doesn't make sense to have 'errors' in AST merging tools, and the changes for ASTImporter part are welcome. However, I don't feel so positive to modules support changes and I don't think we are allowed to change this. @aaron.ballman, @rsmith what do you think?
@bruno, you're the original author of the related Sema code. What do you think, should this be a warning or an error in Sema?

Dec 23 2018, 12:05 AM · Restricted Project

Dec 16 2018

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?

Dec 16 2018, 3:51 PM · Restricted Project

Dec 9 2018

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.

Dec 9 2018, 8:59 AM · Restricted Project
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.

Dec 9 2018, 8:58 AM

Dec 8 2018

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.

Dec 8 2018, 11:50 PM

Dec 6 2018

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

LG!

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

Dec 5 2018

a_sidorin accepted D55133: [CTU] Add statistics.

LGTM with a nit.

Dec 5 2018, 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.

Dec 5 2018, 1:24 PM

Dec 4 2018

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

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

Dec 4 2018, 10:40 PM

Dec 3 2018

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

LGTM, thanks!

Dec 3 2018, 12:30 PM

Dec 1 2018

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

Hi Gabor,
Please find my comments inline.

Dec 1 2018, 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.

Dec 1 2018, 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.

Dec 1 2018, 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?

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

More assertions are always good.

Dec 1 2018, 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.

Dec 1 2018, 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.

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

LGTM, thanks!

Dec 1 2018, 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.

Dec 1 2018, 11:54 AM · Restricted Project
a_sidorin accepted D53708: [ASTImporter] Add importer specific lookup.

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

Dec 1 2018, 11:44 AM

Nov 28 2018

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

Hi Gabor,
Here are some new comments.

Nov 28 2018, 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.

Nov 28 2018, 10:07 PM · Restricted Project

Nov 26 2018

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

LGTM, thanks!

Nov 26 2018, 1:00 PM

Nov 25 2018

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

Thanks!

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

More constants are always welcome.

Nov 25 2018, 11:10 AM
a_sidorin edited reviewers for D53751: [ASTImporter] Added Import functions for transition to new API., added: a_sidorin; removed: a.sidorin.
Nov 25 2018, 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.

Nov 25 2018, 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?

Nov 25 2018, 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.

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

Hi Gabor,

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

LGTM. Thank you for addressing my questions!

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

Hi Gabor,

Nov 25 2018, 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 · Restricted Project
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 · Restricted Project
a_sidorin reopened D44100: [ASTImporter] Reorder fields after structure import is finished.
Nov 18 2018, 2:34 PM · Restricted Project

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 · Restricted Project

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 · Restricted Project

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 · Restricted Project
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