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 (47 w, 2 d)

Recent Activity

Sun, May 19

a_sidorin accepted D62066: [ASTImporter] Enable disabled but passing tests.

Cool!

Sun, May 19, 3:17 PM · Restricted Project, Restricted Project
a_sidorin added a comment to D62064: [ASTImporter] Fix unhandled cases in ASTImporterLookupTable.

Hi Gabor,
This looks fine, but I have a question inline.

Sun, May 19, 3:15 PM · Restricted Project
a_sidorin added inline comments to D61814: [CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *, make it a variant class instead..
Sun, May 19, 12:48 PM · Restricted Project

Sat, May 11

a_sidorin accepted D61814: [CFG] NFC: Remove implicit conversion from CFGTerminator to Stmt *, make it a variant class instead..

The conversion operator indeed looks non-evident.

Sat, May 11, 11:17 AM · Restricted Project
a_sidorin accepted D61815: [CFG] NFC: Modernize test/Analysis/initializers-cfg-output.cpp..

Wow, this is a cool idea!

Sat, May 11, 11:01 AM · Restricted Project
a_sidorin added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Thank you for digging into this! Feel free to ask me if you encounter any problems with the patch.

Sat, May 11, 10:58 AM · Restricted Project
a_sidorin accepted D61786: [ASTImporter] Separate unittest files.

Hi Gabor,
I like this change! LGTM, just a few nits.

Sat, May 11, 10:43 AM · Restricted Project

Sat, May 4

a_sidorin accepted D61545: [analyzer] Fix a crash in RVO from within blocks..
Sat, May 4, 12:23 PM · Restricted Project
a_sidorin accepted D61438: [ASTImporter] Use llvm::Expected and Error in the importer API.

👍

Sat, May 4, 4:13 AM · Restricted Project, Restricted Project, Restricted Project
a_sidorin accepted D61424: [ASTImporter] Fix inequivalence of unresolved exception spec.

Looks good!

Sat, May 4, 3:11 AM · Restricted Project, Restricted Project
a_sidorin accepted D59692: [ASTImporter] Fix name conflict handling.

LGTM, thanks!

Sat, May 4, 2:57 AM · Restricted Project
a_sidorin accepted D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.

Hello Gabor!
This looks good to me, but let's wait for LLDB guys to take a look at the patch. Thanks!

Sat, May 4, 2:33 AM · Restricted Project, Restricted Project

Thu, Apr 25

a_sidorin accepted D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition.

Looks good, thanks!

Thu, Apr 25, 11:55 PM
a_sidorin added a comment to D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition.

Hello Shafik!
The patch itself is fine, but, as other reviewers pointed, tests are appreciated. I suggest to add a test into ASTImporterTests.cpp - you will find several ways to write tests of different complexity here. I think this change can be tested even with the simplest testImport() facility.

Thu, Apr 25, 3:45 PM

Mon, Apr 22

a_sidorin added a comment to D60899: [analyzer] Unbreak body farms in presence of multiple declarations..

Mmm, what are the pros and cons?

Mon, Apr 22, 4:32 PM · Restricted Project, Restricted Project

Apr 20 2019

a_sidorin accepted D60899: [analyzer] Unbreak body farms in presence of multiple declarations..

Hi Artem,
This looks good to me. However, it seems to me that the correct solution is to synthesize a shining new function and include it into the redeclaration chain. This requires much more work, however.

Apr 20 2019, 3:05 AM · Restricted Project, Restricted Project

Apr 16 2019

a_sidorin added a comment to D60796: [analyzer] PR41269: SmartPtrModeling..

Hi Artem,
There are some comments inline, but looks promising!

Apr 16 2019, 3:03 PM · Restricted Project
a_sidorin accepted D60742: [analyzer] RegionStore: Enable loading default bindings from variables..

I like the test even more than the change itself!

Apr 16 2019, 2:56 AM · Restricted Project
a_sidorin accepted D60739: [analyzer] NFC: Re-use reusable unittest mocks..

LGTM.

Apr 16 2019, 2:48 AM · Restricted Project, Restricted Project

Apr 8 2019

a_sidorin accepted D55049: Changed every use of ASTImporter::Import to Import_New.

LGTM.

Apr 8 2019, 1:42 AM · Restricted Project

Mar 28 2019

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

Hello Raphael,
I think we should accept this change. I don't see an easy way to merge the LLDB stuff into ASTImporter; also, this patch provides a good extension point for ASTImporter since it is designed to be a parent class. @martong @shafik Gabor, Shafik, what do you think?

Mar 28 2019, 4:33 PM · Restricted Project
a_sidorin accepted D59761: [ASTImporter] Convert ODR diagnostics inside ASTImporter implementation.

Yes, I think this is fine. Thanks!

Mar 28 2019, 4:29 PM · Restricted Project, Restricted Project
a_sidorin accepted D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName.

Thanks!

Mar 28 2019, 3:53 PM · Restricted Project
a_sidorin accepted D58897: [ASTImporter] Make ODR error handling configurable.
Mar 28 2019, 3:53 PM · Restricted Project
a_sidorin added a comment to D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it.

Post-LGTM with some stylish nits.

Mar 28 2019, 2:52 PM · Restricted Project
a_sidorin added a comment to D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName.

Hi Shafik,
Thank you for the explanation, it is much more clear to me now. But, as I see, D59692 is going to discard the changes this patch introduces. @martong Gabor, do you expect the changes of this patch to be merged into yours, or should this patch be abandoned?

Mar 28 2019, 2:42 PM · Restricted Project
a_sidorin added a comment to D59692: [ASTImporter] Fix name conflict handling.

Thanks for the fixes! I'd like to clarify one moment, however.

Mar 28 2019, 2:29 PM · Restricted Project

Mar 24 2019

a_sidorin added a comment to D55049: Changed every use of ASTImporter::Import to Import_New.

Hi Balazs,

Mar 24 2019, 12:23 PM · Restricted Project
a_sidorin accepted D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter..

Hi Balasz,

Mar 24 2019, 9:53 AM · Restricted Project, Restricted Project
a_sidorin added a comment to D59485: [ASTImporter] Add an ImportImpl method to allow customizing Import behavior..

Hello Raphael,

Mar 24 2019, 9:30 AM · Restricted Project
a_sidorin added a comment to D59692: [ASTImporter] Fix name conflict handling.

Hi Gabor,

Mar 24 2019, 9:13 AM · Restricted Project

Mar 21 2019

a_sidorin updated subscribers of D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName.

Hi Shafik,

Mar 21 2019, 6:42 PM · Restricted Project

Mar 11 2019

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.

Mar 11 2019, 12:34 AM · Restricted Project

Mar 10 2019

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

Thanks!

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

Looks good!

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

Mar 3 2019

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.

Mar 3 2019, 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.

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

Feb 26 2019

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

Thanks for the fixes!

Feb 26 2019, 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?

Feb 26 2019, 1:59 PM · Restricted Project

Feb 24 2019

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.

Feb 24 2019, 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.

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

Feb 23 2019

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.

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

Feb 21 2019

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

Feb 20 2019

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

Hi Tom,

Feb 20 2019, 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