Page MenuHomePhabricator

shafik (Shafik Yaghmour)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 12 2018, 2:31 PM (44 w, 4 d)

Recent Activity

Wed, May 15

shafik added a comment to D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.

I ran check-lldb and I hit one regression in TestFormatters.py, part of what I am seeing is as follows:

Wed, May 15, 2:00 PM · Restricted Project, Restricted Project
shafik added a comment to D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo().

@friss we have several bugs, once of which I can reproduce but I have not been able to reduce it to a minimal case yet and the nullptr check is obviously the right to do.

Wed, May 15, 12:01 PM
shafik updated the diff for D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo().

Simplified the checking of symbol being a nullptr

Wed, May 15, 11:36 AM

Tue, May 14

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

Done with first round.

Tue, May 14, 10:17 AM · Restricted Project

Mon, May 13

shafik committed rG9acacebf83d3: [DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses… (authored by shafik).
[DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses…
Mon, May 13, 9:46 AM
shafik committed rLLDB360599: [DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses….
[DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses…
Mon, May 13, 9:46 AM
shafik committed rL360599: [DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses….
[DataFormatters] FindLibCppStdFunctionCallableInfo() currently uses…
Mon, May 13, 9:46 AM
shafik closed D61759: Switch to FindSymbolsMatchingRegExAndType() from FindFunctions() in FindLibCppStdFunctionCallableInfo().
Mon, May 13, 9:46 AM · Restricted Project

Fri, May 10

shafik edited reviewers for D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo(), added: aprantl; removed: aam.
Fri, May 10, 2:32 PM
shafik created D61805: Add nullptr check in FindLibCppStdFunctionCallableInfo().
Fri, May 10, 2:29 PM

Thu, May 9

shafik created D61759: Switch to FindSymbolsMatchingRegExAndType() from FindFunctions() in FindLibCppStdFunctionCallableInfo().
Thu, May 9, 1:53 PM · Restricted Project

Fri, May 3

shafik committed rGe5cbe78259c9: Fix for ambiguous lookup in expressions between local variable and namespace (authored by shafik).
Fix for ambiguous lookup in expressions between local variable and namespace
Fri, May 3, 12:59 PM
shafik committed rLLDB359921: Fix for ambiguous lookup in expressions between local variable and namespace.
Fix for ambiguous lookup in expressions between local variable and namespace
Fri, May 3, 12:58 PM
shafik committed rL359921: Fix for ambiguous lookup in expressions between local variable and namespace.
Fix for ambiguous lookup in expressions between local variable and namespace
Fri, May 3, 12:58 PM
shafik closed D59960: Fix for ambiguous lookup in expressions between local variable and namespace.
Fri, May 3, 12:58 PM · Restricted Project
shafik updated the diff for D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

Updating after https://reviews.llvm.org/D46551 landed

Fri, May 3, 12:29 PM · Restricted Project

Thu, May 2

shafik added a comment to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

@teemperor @jingham @clayborg I believe now that https://reviews.llvm.org/D46551 is landed the performance concerns should be addressed.

Thu, May 2, 3:09 PM · Restricted Project
shafik added a comment to D61440: C.128 override, virtual keyword handling.

Thank you! LGTM, in general we avoid "large" refactoring changes to avoid polluting the blame list but the changes are relatively local and they are good changes that can catch real bugs in the future. I would like a second set of eyes though.

Thu, May 2, 10:15 AM · Restricted Project, Restricted Project
shafik added reviewers for D61440: C.128 override, virtual keyword handling: teemperor, JDevlieghere, davide, shafik.
Thu, May 2, 9:24 AM · Restricted Project, Restricted Project

Wed, May 1

shafik committed rG2097b1f84d47: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference… (authored by shafik).
Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference…
Wed, May 1, 3:22 PM
shafik committed rLLDB359732: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference….
Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference…
Wed, May 1, 3:22 PM
shafik committed rL359732: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference….
Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference…
Wed, May 1, 3:22 PM
shafik closed D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.
Wed, May 1, 3:22 PM · Restricted Project
shafik added inline comments to D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.
Wed, May 1, 2:05 PM · Restricted Project
shafik updated the diff for D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

Modifying copy contructor to be act more intuitively.

Wed, May 1, 2:05 PM · Restricted Project
shafik updated the diff for D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

Testing both passing as and argument and returning

Wed, May 1, 1:31 PM · Restricted Project
shafik added a comment to D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

@friss added second test

Wed, May 1, 1:31 PM · Restricted Project
shafik added a comment to D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

@teemperor good call, that is indeed simpler and yes I did not intend that delete.

Wed, May 1, 12:53 PM · Restricted Project
shafik updated the diff for D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.
  • Simplifying test
  • Fixing unintended deleted test
Wed, May 1, 12:53 PM · Restricted Project
shafik committed rGc3dd67204c92: Disabling test in TestClassTemplateParameterPack.py until we do template lookup… (authored by shafik).
Disabling test in TestClassTemplateParameterPack.py until we do template lookup…
Wed, May 1, 9:39 AM
shafik committed rLLDB359699: Disabling test in TestClassTemplateParameterPack.py until we do template lookup….
Disabling test in TestClassTemplateParameterPack.py until we do template lookup…
Wed, May 1, 9:39 AM
shafik committed rL359699: Disabling test in TestClassTemplateParameterPack.py until we do template lookup….
Disabling test in TestClassTemplateParameterPack.py until we do template lookup…
Wed, May 1, 9:39 AM
shafik added a comment to D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

@aprantl @teemperor I believe I addressed your comments.

Wed, May 1, 8:58 AM · Restricted Project

Tue, Apr 30

shafik accepted D61299: Rename Minion to ASTImporterDelegate.

This is a good change!

Tue, Apr 30, 2:45 PM · Restricted Project, Restricted Project
shafik updated the diff for D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

Changed to reflect comments.

  • Added comments to test to explain what it is doing.
  • Formatting and other minor fixes.
Tue, Apr 30, 1:24 PM · Restricted Project
shafik added inline comments to D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.
Tue, Apr 30, 1:24 PM · Restricted Project
shafik accepted D61305: Add std::stack and std::queue support to CxxModuleHandler.

LGTM

Tue, Apr 30, 10:01 AM · Restricted Project, Restricted Project, Restricted Project
shafik added a comment to D61266: Skip TestClassTemplateParameterPack.py on all platforms.

@friss makes sense, updated comment.

Tue, Apr 30, 9:47 AM
shafik updated the diff for D61266: Skip TestClassTemplateParameterPack.py on all platforms.

Updated comment to be more precise.

Tue, Apr 30, 9:47 AM

Mon, Apr 29

shafik updated the diff for D61266: Skip TestClassTemplateParameterPack.py on all platforms.

Fred is correct, I mistakenly thought the parts of the test that were working were being covered elsewhere but that is not the case. So I have reworked this change to instead of skipping the whole test to comment out the inline expressions that are specifically broken.

Mon, Apr 29, 2:03 PM
shafik added a comment to D61266: Skip TestClassTemplateParameterPack.py on all platforms.

@friss updated the change to only effect those specifically broken.

Mon, Apr 29, 2:03 PM
shafik created D61266: Skip TestClassTemplateParameterPack.py on all platforms.
Mon, Apr 29, 9:27 AM

Fri, Apr 26

shafik committed rG16b90733c751: [ASTImporter] Copy Argument Passing Restrictions setting when importing a… (authored by shafik).
[ASTImporter] Copy Argument Passing Restrictions setting when importing a…
Fri, Apr 26, 11:51 AM
shafik committed rL359338: [ASTImporter] Copy Argument Passing Restrictions setting when importing a….
[ASTImporter] Copy Argument Passing Restrictions setting when importing a…
Fri, Apr 26, 11:49 AM
shafik committed rC359338: [ASTImporter] Copy Argument Passing Restrictions setting when importing a….
[ASTImporter] Copy Argument Passing Restrictions setting when importing a…
Fri, Apr 26, 11:49 AM

Thu, Apr 25

shafik updated the diff for D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition.

Added test

Thu, Apr 25, 4:41 PM
shafik added a comment to D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.

@rsmith I tagged you in this change in case we are missing any implications in using DW_CC_pass_by_reference to do setArgPassingRestrictions(clang::RecordDecl::APK_CannotPassInRegs);

Thu, Apr 25, 1:46 PM · Restricted Project
shafik updated the summary of D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition.
Thu, Apr 25, 1:42 PM
shafik updated the summary of D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.
Thu, Apr 25, 1:42 PM · Restricted Project
shafik created D61146: Set a CXXRecordDecl to not be passed in registers if DW_CC_pass_by_reference when loading from DWARF.
Thu, Apr 25, 1:41 PM · Restricted Project
shafik created D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition.
Thu, Apr 25, 11:02 AM

Wed, Apr 24

shafik committed rGabdb816b776c: [DataFormatters] Adjusting libc++ std::list formatter to act better with… (authored by shafik).
[DataFormatters] Adjusting libc++ std::list formatter to act better with…
Wed, Apr 24, 10:37 AM
shafik committed rLLDB359118: [DataFormatters] Adjusting libc++ std::list formatter to act better with….
[DataFormatters] Adjusting libc++ std::list formatter to act better with…
Wed, Apr 24, 10:37 AM
shafik committed rL359118: [DataFormatters] Adjusting libc++ std::list formatter to act better with….
[DataFormatters] Adjusting libc++ std::list formatter to act better with…
Wed, Apr 24, 10:37 AM
shafik closed D60588: Adjusting libc++ std::list formatter to act better with pointers and references and adding a test to cover a previous related fix.
Wed, Apr 24, 10:36 AM · Restricted Project
shafik added inline comments to D61046: Fix compilation warnings when compiling with GCC 7.3.
Wed, Apr 24, 9:58 AM · lld, Restricted Project, Restricted Project

Apr 15 2019

shafik committed rLLDB358462: [ASTImporter] Regression test to ensure that we handling importing of anonymous….
[ASTImporter] Regression test to ensure that we handling importing of anonymous…
Apr 15 2019, 4:06 PM
shafik committed rGe4b19c9c2868: [ASTImporter] Regression test to ensure that we handling importing of anonymous… (authored by shafik).
[ASTImporter] Regression test to ensure that we handling importing of anonymous…
Apr 15 2019, 4:05 PM
shafik committed rL358462: [ASTImporter] Regression test to ensure that we handling importing of anonymous….
[ASTImporter] Regression test to ensure that we handling importing of anonymous…
Apr 15 2019, 4:03 PM
shafik closed D59667: Regression test to ensure that we handling importing of anonymous enums correctly.
Apr 15 2019, 4:03 PM · Restricted Project
shafik added inline comments to D59667: Regression test to ensure that we handling importing of anonymous enums correctly.
Apr 15 2019, 3:05 PM · Restricted Project
shafik updated the diff for D59667: Regression test to ensure that we handling importing of anonymous enums correctly.

Small updated to test, remove use of printf and associated include.

Apr 15 2019, 3:05 PM · Restricted Project

Apr 11 2019

shafik added inline comments to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.
Apr 11 2019, 2:55 PM · Restricted Project
shafik created D60588: Adjusting libc++ std::list formatter to act better with pointers and references and adding a test to cover a previous related fix.
Apr 11 2019, 2:55 PM · Restricted Project

Apr 10 2019

shafik added a comment to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

@friss I had to rework the tests a little but they now cover Objective-C static and non-static methods as well as C and C++.

Apr 10 2019, 3:42 PM · Restricted Project
shafik updated the diff for D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

-Adjusting tests to ensure coverage of Objecive-C static and non-static methods and C and C++

Apr 10 2019, 3:37 PM · Restricted Project

Apr 9 2019

shafik added inline comments to D59537: Instantiate 'std' templates explicitly in the expression evaluator.
Apr 9 2019, 1:27 PM · Restricted Project, Restricted Project, Restricted Project
shafik added inline comments to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.
Apr 9 2019, 12:13 PM · Restricted Project
shafik added a comment to D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

@friss I believe I have addressed your comments

Apr 9 2019, 12:12 PM · Restricted Project
shafik updated the diff for D59960: Fix for ambiguous lookup in expressions between local variable and namespace.

Addressing comments:

  • Now applies to all languages not just C++
  • When adding locals be more selective on filtering i.e. only filter self and _cmd for Objective C etc...
Apr 9 2019, 12:12 PM · Restricted Project

Apr 8 2019

shafik committed rGd4263123abfc: [ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using… (authored by shafik).
[ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using…
Apr 8 2019, 1:49 PM
shafik committed rL357940: [ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using….
[ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using…
Apr 8 2019, 1:48 PM
shafik committed rC357940: [ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using….
[ASTImporter] Call to HandleNameConflict in VisitEnumDecl mistakeningly using…
Apr 8 2019, 1:48 PM
shafik closed D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName.
Apr 8 2019, 1:48 PM · Restricted Project

Mar 28 2019

shafik accepted D59761: [ASTImporter] Convert ODR diagnostics inside ASTImporter implementation.

LGTM

Mar 28 2019, 5:23 PM · Restricted Project, Restricted Project
shafik created D59960: Fix for ambiguous lookup in expressions between local variable and namespace.
Mar 28 2019, 2:06 PM · Restricted Project
shafik added a comment to D59847: Regression test to ensure that we handling importing of std::vector of enums correctly.

@stella.stamenova I committed a fix, please let me know if this does not address the regression:

Mar 28 2019, 1:26 PM · Restricted Project
shafik committed rGa1f1ff889639: Fix for regression test, since we rely on the formatter for std::vector in the… (authored by shafik).
Fix for regression test, since we rely on the formatter for std::vector in the…
Mar 28 2019, 1:25 PM
shafik committed rLLDB357210: Fix for regression test, since we rely on the formatter for std::vector in the….
Fix for regression test, since we rely on the formatter for std::vector in the…
Mar 28 2019, 1:25 PM
shafik committed rL357210: Fix for regression test, since we rely on the formatter for std::vector in the….
Fix for regression test, since we rely on the formatter for std::vector in the…
Mar 28 2019, 1:25 PM
shafik added a comment to D59847: Regression test to ensure that we handling importing of std::vector of enums correctly.

This is causing failures on the windows bot. Please fix it or revert it.

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/3066

Mar 28 2019, 1:17 PM · Restricted Project
shafik committed rLLDB357188: Regression test to ensure that we handling importing of std::vector of enums….
Regression test to ensure that we handling importing of std::vector of enums…
Mar 28 2019, 10:22 AM
shafik committed rG0f71a25e985a: Regression test to ensure that we handling importing of std::vector of enums… (authored by shafik).
Regression test to ensure that we handling importing of std::vector of enums…
Mar 28 2019, 10:21 AM
shafik committed rL357188: Regression test to ensure that we handling importing of std::vector of enums….
Regression test to ensure that we handling importing of std::vector of enums…
Mar 28 2019, 10:20 AM
shafik closed D59847: Regression test to ensure that we handling importing of std::vector of enums correctly.
Mar 28 2019, 10:20 AM · Restricted Project

Mar 27 2019

shafik added a comment to D59847: Regression test to ensure that we handling importing of std::vector of enums correctly.

@friss so far I have not found any other scenarios that relate specifically to this test outside of the one I already have here https://reviews.llvm.org/D59667

Mar 27 2019, 4:27 PM · Restricted Project
shafik committed rGe5094d6d3d29: [ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re… (authored by shafik).
[ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re…
Mar 27 2019, 10:47 AM
shafik committed rC357100: [ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re….
[ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re…
Mar 27 2019, 10:47 AM
shafik committed rL357100: [ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re….
[ASTImporter] Fix IsStructuralMatch specialization for EnumDecl to prevent re…
Mar 27 2019, 10:46 AM
shafik closed D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it.
Mar 27 2019, 10:46 AM · Restricted Project
shafik added a comment to D59847: Regression test to ensure that we handling importing of std::vector of enums correctly.

As we're just adding test coverage, could we add a little more?

  • Anonymous enum
  • Enum through a typedef
  • class enum
  • enum declared inside of the function rather than at the top-level
  • nested enum in a record type
  • enum nested in a templated class
  • anything else I haven't thought about...
Mar 27 2019, 10:07 AM · Restricted Project
shafik added inline comments to D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it.
Mar 27 2019, 10:01 AM · Restricted Project
shafik updated the diff for D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it.

Fixes based on comments.

Mar 27 2019, 10:01 AM · Restricted Project

Mar 26 2019

shafik added a comment to D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it.

LLDB regression test that goes with this fix: https://reviews.llvm.org/D59847

Mar 26 2019, 2:32 PM · Restricted Project
shafik created D59847: Regression test to ensure that we handling importing of std::vector of enums correctly.
Mar 26 2019, 2:28 PM · Restricted Project
shafik created D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it.
Mar 26 2019, 2:11 PM · Restricted Project
shafik added a comment to rL357010: [coroutines] Add std::experimental::task<T> type.

It looks like this breaks the lldb build bots, more specifically it looks like all modules builds are broken, see the following log:

Mar 26 2019, 12:43 PM
shafik added a comment to D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName.

@martong your idea does not work b/c default construction DeclarationName() treats it the same as being empty. So if (!Name) is still true.

And did you try moving the push_back to the other scope? I'd expect the the ConflictingDecls to be empty then.

Mar 26 2019, 10:10 AM · Restricted Project

Mar 25 2019

shafik added a comment to D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName.

@martong your idea does not work b/c default construction DeclarationName() treats it the same as being empty. So if (!Name) is still true.

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

Hi Shafik,

Thank you for looking into this. This is indeed a bug, because whenever a we encounter an unnamed EnumDecl in the "from" context then we return with a nameconflict error.
However, I think we should fix this differently.
First of all, currently HandleNameConflict just returns the parameter Name it received. This may be unnamed in some cases and thus converts to true. So I think HandleNameConflict should be changed that it would return a default initialized DeclarationName; so once we have anything in the ConflictingDecls then we return with the NameConflict error:

DeclarationName ASTImporter::HandleNameConflict(DeclarationName Name,
                                                DeclContext *DC,
                                                unsigned IDNS,
                                                NamedDecl **Decls,
                                                unsigned NumDecls) {
-  return Name;
+  return DeclarationName();
}

And most importantly, I think we should push into the ConflictingDecls only if the structural match indicates a non-match:

      if (auto *FoundEnum = dyn_cast<EnumDecl>(FoundDecl)) {
        if (isStructuralMatch(D, FoundEnum, true))
          return Importer.MapImported(D, FoundEnum);
+       ConflictingDecls.push_back(FoundDecl);
      }
-
-     ConflictingDecls.push_back(FoundDecl);

I am aware of similar errors like this with other AST nodes. We had a patch in our fork to fix that in January (https://github.com/Ericsson/clang/pull/569/files) I am going to prepare a patch from that cause I see now this affects you guys in LLDB too.

Mar 25 2019, 9:08 AM · Restricted Project