Page MenuHomePhabricator

shafik (Shafik Yaghmour)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 12 2018, 2:31 PM (62 w, 3 d)

Recent Activity

Tue, Sep 10

shafik added inline comments to D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies.
Tue, Sep 10, 10:30 PM · Restricted Project, Restricted Project
shafik added inline comments to D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies.
Tue, Sep 10, 10:14 PM · Restricted Project, Restricted Project
shafik added inline comments to D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies.
Tue, Sep 10, 9:53 PM · Restricted Project, Restricted Project
shafik updated the diff for D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.

Changes based on comments:

Tue, Sep 10, 4:59 PM
shafik added inline comments to D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.
Tue, Sep 10, 4:07 PM
shafik updated the diff for D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.
  • Fixing test based on comments
Tue, Sep 10, 4:07 PM
shafik updated the diff for D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.
  • Refactored test to use a function to do repetitive task
Tue, Sep 10, 2:15 PM

Fri, Sep 6

shafik committed rL371268: Request commit access for shafik.
Request commit access for shafik
Fri, Sep 6, 3:52 PM
shafik added inline comments to D67227: [lldb] Extend and document TestIRInterpreter.py.
Fri, Sep 6, 10:17 AM · Restricted Project

Thu, Sep 5

shafik added inline comments to D67227: [lldb] Extend and document TestIRInterpreter.py.
Thu, Sep 5, 2:23 PM · Restricted Project
shafik added a comment to D67236: [MC] Fix undefined behavior in MCInstPrinter::formatHex.

It is not obvious what the change in result is expected to be so a test would be helpful.

Thu, Sep 5, 1:03 PM · Restricted Project
shafik added inline comments to D67227: [lldb] Extend and document TestIRInterpreter.py.
Thu, Sep 5, 9:30 AM · Restricted Project

Wed, Sep 4

shafik updated the diff for D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.

Addressing comments

Wed, Sep 4, 3:23 PM
shafik accepted D67174: Rename of constants in ASTImporterVisibilityTest. NFC..

Thank you! LGTM

Wed, Sep 4, 10:22 AM · Restricted Project, Restricted Project

Tue, Sep 3

shafik accepted D64480: [ASTImporter] Added visibility context check for TypedefNameDecl..
Tue, Sep 3, 2:32 PM · Restricted Project, Restricted Project
shafik added inline comments to D64480: [ASTImporter] Added visibility context check for TypedefNameDecl..
Tue, Sep 3, 2:32 PM · Restricted Project, Restricted Project
shafik added a comment to D64480: [ASTImporter] Added visibility context check for TypedefNameDecl..

It is worth noting that:

typedef int T;
typedef int T;

is not valid C99 see godbolt

Should we handle this case? This can be special for C99 only when the declarations must be merged instead of linked. Probably this does not cause functional problems if we leave it as is.

Tue, Sep 3, 2:29 PM · Restricted Project, Restricted Project
shafik created D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.
Tue, Sep 3, 10:34 AM

Sat, Aug 31

shafik added a comment to D64480: [ASTImporter] Added visibility context check for TypedefNameDecl..

It is worth noting that:

Sat, Aug 31, 6:36 AM · Restricted Project, Restricted Project

Fri, Aug 30

shafik added inline comments to D66999: [ASTImporter][NFC] Add comments to code where we cannot use HandleNameConflict.
Fri, Aug 30, 3:28 PM · Restricted Project
shafik added inline comments to D64480: [ASTImporter] Added visibility context check for TypedefNameDecl..
Fri, Aug 30, 12:46 PM · Restricted Project, Restricted Project

Thu, Aug 29

shafik accepted D66933: [ASTImporter] Propagate errors during import of overridden methods..

LGTM but I agree w/ Gabor's comments

Thu, Aug 29, 9:40 PM · Restricted Project, Restricted Project
shafik accepted D66348: [ASTImporter] Do not look up lambda classes.

I was concerned about how this would affect LLDB but after thinking about it I realized that in the DWARF we will just end up with one DW_TAG_class_type.

Thu, Aug 29, 9:17 PM · Restricted Project, Restricted Project

Tue, Aug 27

shafik committed rL370107: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Debug Info: Support for DW_AT_export_symbols for anonymous structs
Tue, Aug 27, 1:25 PM
shafik committed rG528f5da6d862: Debug Info: Support for DW_AT_export_symbols for anonymous structs (authored by shafik).
Debug Info: Support for DW_AT_export_symbols for anonymous structs
Tue, Aug 27, 1:19 PM
shafik closed D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Tue, Aug 27, 1:19 PM · Restricted Project, debug-info
shafik updated the diff for D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

Simplifying test.

Tue, Aug 27, 11:44 AM · Restricted Project, debug-info
shafik added inline comments to D66791: [lldb][ELF] Read symbols from .gnu_debugdata sect..
Tue, Aug 27, 11:01 AM · Restricted Project, Restricted Project

Mon, Aug 26

shafik updated the diff for D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

Updating test to be more specific

Mon, Aug 26, 4:55 PM · Restricted Project, debug-info
shafik accepted D59692: [ASTImporter] Fix name conflict handling with different strategies.

Other than my two comment this LGTM

Mon, Aug 26, 4:13 PM · Restricted Project, Restricted Project
shafik added inline comments to D66739: [lldb] Add -w flag for expressions to print compiler warnings even if expression succeeds..
Mon, Aug 26, 2:11 PM · Restricted Project
shafik committed rL369969: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Debug Info: Support for DW_AT_export_symbols for anonymous structs
Mon, Aug 26, 2:07 PM
shafik committed rG90e00bd8f3e1: Debug Info: Support for DW_AT_export_symbols for anonymous structs (authored by shafik).
Debug Info: Support for DW_AT_export_symbols for anonymous structs
Mon, Aug 26, 2:01 PM
shafik closed D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Mon, Aug 26, 2:01 PM · Restricted Project, debug-info

Aug 23 2019

shafik added a parent revision for D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs: D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 23 2019, 10:55 AM · Restricted Project, debug-info
shafik added a child revision for D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs: D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 23 2019, 10:55 AM · Restricted Project, debug-info
shafik created D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 23 2019, 10:55 AM · Restricted Project, debug-info
shafik committed rG5dca5efc0b14: Debug Info: Support for DW_AT_export_symbols for anonymous structs (authored by shafik).
Debug Info: Support for DW_AT_export_symbols for anonymous structs
Aug 23 2019, 10:19 AM
shafik committed rL369781: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Debug Info: Support for DW_AT_export_symbols for anonymous structs
Aug 23 2019, 10:19 AM
shafik closed D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 23 2019, 10:19 AM · Restricted Project, debug-info

Aug 22 2019

shafik updated the diff for D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
  • Adding more context
  • Adjusting the test to be more strict
Aug 22 2019, 2:08 PM · Restricted Project, debug-info
shafik added a child revision for D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs: D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 22 2019, 10:50 AM · Restricted Project, debug-info
shafik added a parent revision for D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs: D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 22 2019, 10:50 AM · Restricted Project, debug-info
shafik created D66605: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 22 2019, 10:50 AM · Restricted Project, debug-info

Aug 21 2019

shafik updated the diff for D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
  • Adding documentation to LangRef.rst
  • Simplified the test further.
Aug 21 2019, 3:07 PM · Restricted Project, debug-info
shafik added a comment to D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

@djtodoro Thank for the suggestions on how to simplify the test even more.

Aug 21 2019, 3:05 PM · Restricted Project, debug-info
shafik added a comment to D66447: Add char8_t support (C++20).

This looks good to me, but why are we using a nul character to test utf8 support? Shouldn't we insert some funnier characters too? I mean, one of the advantages of unicode is that it should not be affected by the system code pages and such, so hopefully this would not cause problems even on some more exotic setups. (And I am pretty sure I remember already seeing some chinese chars in some of our data formatter tests)

I only glanced at the proposal, but unless I misunderstand the type only fits UTF-8 characters representable in 1 byte, which are basically just ASCII.

I have now too glanced at the proposal (just the cppreference page, really :) ). I think I understand where you got this impression from, but I don't think that is fully correct. It is true that a *single* char8_t variable can hold only 8 bit UTF8 code units (*not* characters), but that is not surprising since UTF8 is a variable length encoding, so you can't have a type that matches one character exactly. However, an *array* of char8_t is a completely different thing, and I am pretty sure that these are intended to hold utf8 strings containing any utf8 characters (otherwise, it wouldn't really deserve to call itself a utf8 type), and so we should print (and test) it as regular utf8.

However, this actually surfaces the question of how should we format single char8_t variables. It makes sense to display the character value if the value happens to be ASCII, but I guess we shouldn't print something like "invalid utf8 character" if it does contain one unit of the multibyte characters.

Aug 21 2019, 9:21 AM · Restricted Project, Restricted Project

Aug 20 2019

shafik updated the diff for D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

Changed isExportSymbols -> getExportSymbols

Aug 20 2019, 5:07 PM · Restricted Project, debug-info
shafik added inline comments to D66250: [JIT][Unwinder] Add Trampoline ObjectFile and UnwindPlan support for FCB.
Aug 20 2019, 12:03 PM · Restricted Project
shafik added inline comments to D66248: [JIT][Command] Add "inject-condition" flag to conditional breakpoints.
Aug 20 2019, 11:53 AM · Restricted Project
shafik added a comment to D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

I also just realized that although I originally talked about LLDB being the consumer, ultimately since we have to assume any AST we generate from DWARF can be used in expression parsing clang is also the consumer as well.

Aug 20 2019, 8:34 AM · Restricted Project, debug-info

Aug 19 2019

shafik added a comment to D66348: [ASTImporter] Do not look up lambda classes.

I am not enthusiastic about this solution but I need to think about it some more.

Aug 19 2019, 9:55 PM · Restricted Project, Restricted Project
shafik added a reviewer for D66348: [ASTImporter] Do not look up lambda classes: teemperor.
Aug 19 2019, 9:45 PM · Restricted Project, Restricted Project
shafik added a comment to D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

The difference between those two cases would be based on their usage - the types are the same (kinda, basically). In one case there's an anonymous member of the anonymous type (so it's transparent/all the nested names are as-if they were names of members in the outer type) and in the other case there's a named variable of the anonymous type (so the named members are members of that outer member variable).

Aug 19 2019, 9:11 PM · Restricted Project, debug-info
shafik added a comment to D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

Eh, I think it's still pretty unnecessary (& the DWARF spec is only suggestive, doesn't tend to have requirements in this regard) and so I'd probably suggest waiting until some consumer really wants this (& I'd still want to have a discussion with the consumer about why they find this to be needed). But it's really cheap (especially in DWARFv5 where it can use a const_value form & cost no bytes in debug_info (if/when that sort of thing is implemented in LLVM, if it isn't already).

@shafik, Can you outline the problem Clang has differentiating anonymous structs and lambdas that made this patch necessary?

Aug 19 2019, 4:09 PM · Restricted Project, debug-info

Aug 16 2019

shafik updated the diff for D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.

Deleting some lines from the test as requested.

Aug 16 2019, 9:17 AM · Restricted Project, debug-info
shafik created D66352: Debug Info: Support for DW_AT_export_symbols for anonymous structs.
Aug 16 2019, 8:09 AM · Restricted Project, debug-info

Aug 15 2019

shafik added a comment to D59692: [ASTImporter] Fix name conflict handling with different strategies.

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 issues w/ bugs like that but I really would like to see more tests. Are you having issues coming up w/ tests?

Aug 15 2019, 1:57 PM · Restricted Project, Restricted Project
shafik accepted D65935: [ASTImporter] Import ctor initializers after setting flags..

I was hoping to be able reproduce this in LLDB via an expression like this:

Aug 15 2019, 1:18 PM · Restricted Project, Restricted Project
shafik added inline comments to D66248: [JIT][Command] Add "inject-condition" flag to conditional breakpoints.
Aug 15 2019, 11:05 AM · Restricted Project

Aug 14 2019

shafik committed rG62abe494fb36: Improve anonymous class heuristic in ClangASTContext::CreateRecordType (authored by shafik).
Improve anonymous class heuristic in ClangASTContext::CreateRecordType
Aug 14 2019, 3:31 PM
shafik committed rL368937: Improve anonymous class heuristic in ClangASTContext::CreateRecordType.
Improve anonymous class heuristic in ClangASTContext::CreateRecordType
Aug 14 2019, 3:29 PM
shafik closed D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType .
Aug 14 2019, 3:29 PM · Restricted Project
shafik updated the diff for D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType .

Fixing has_name to better reflect the condition.

Aug 14 2019, 2:15 PM · Restricted Project
shafik added a comment to D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType .

@aprantl @teemperor Good catches, I have updated the review.

Aug 14 2019, 1:36 PM · Restricted Project
shafik updated the diff for D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType .

Addressing comments:

Aug 14 2019, 1:36 PM · Restricted Project

Aug 13 2019

shafik created D66175: Improve anonymous class heuristic in ClangASTContext::CreateRecordType .
Aug 13 2019, 3:13 PM · Restricted Project

Aug 12 2019

shafik committed rGcb282b4ebcc6: [ASTDump] Add is_anonymous to VisitCXXRecordDecl (authored by shafik).
[ASTDump] Add is_anonymous to VisitCXXRecordDecl
Aug 12 2019, 10:11 AM
shafik committed rL368591: [ASTDump] Add is_anonymous to VisitCXXRecordDecl.
[ASTDump] Add is_anonymous to VisitCXXRecordDecl
Aug 12 2019, 10:08 AM
shafik closed D66028: [ASTDump] Add is_anonymous to VisitCXXRecordDecl .
Aug 12 2019, 10:07 AM · Restricted Project

Aug 9 2019

shafik created D66028: [ASTDump] Add is_anonymous to VisitCXXRecordDecl .
Aug 9 2019, 1:23 PM · Restricted Project

Aug 2 2019

shafik committed rGfa5c340ea12e: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl (authored by shafik).
Fix ClangASTContext::CreateParameterDeclaration to not call addDecl
Aug 2 2019, 2:43 PM
shafik committed rL367726: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.
Fix ClangASTContext::CreateParameterDeclaration to not call addDecl
Aug 2 2019, 2:43 PM
shafik closed D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.
Aug 2 2019, 2:43 PM · Restricted Project
shafik committed rGc5d401453553: [Formatters] Temporarily disable libc++ std::function formatter due to… (authored by shafik).
[Formatters] Temporarily disable libc++ std::function formatter due to…
Aug 2 2019, 11:18 AM
shafik committed rL367701: [Formatters] Temporarily disable libc++ std::function formatter due to….
[Formatters] Temporarily disable libc++ std::function formatter due to…
Aug 2 2019, 11:15 AM
shafik closed D65666: Temporarily disable libc++ std::function formatter due to performance issue.
Aug 2 2019, 11:15 AM · Restricted Project
shafik added a comment to D65666: Temporarily disable libc++ std::function formatter due to performance issue.

@aprantl I did not know either until I looked at decorators.py and it says:

Aug 2 2019, 11:04 AM · Restricted Project
shafik updated the diff for D65666: Temporarily disable libc++ std::function formatter due to performance issue.

Adding as suggested.

Aug 2 2019, 11:02 AM · Restricted Project
shafik created D65666: Temporarily disable libc++ std::function formatter due to performance issue.
Aug 2 2019, 10:55 AM · Restricted Project

Aug 1 2019

shafik added a comment to D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

@labath @stella.stamenova I updated the diff with a fix that I believe should address the test failure.

Aug 1 2019, 12:48 PM · Restricted Project
shafik updated the diff for D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

Fix that should fix the failing PDB test.

Aug 1 2019, 12:42 PM · Restricted Project
shafik added a comment to D65573: Add User docs for ASTImporter.

Thank you for writing this up! I just have a few minor comments.

Aug 1 2019, 12:40 PM · Restricted Project, Restricted Project
shafik added a reviewer for D65573: Add User docs for ASTImporter: teemperor.
Aug 1 2019, 9:52 AM · Restricted Project, Restricted Project
shafik added inline comments to D65577: [ASTImporter] Import default expression of param before creating the param..
Aug 1 2019, 9:47 AM · Restricted Project

Jul 30 2019

shafik added a comment to D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

@stella.stamenova that is unfortunate but not surprising. I don't have a way to test a fix locally. Is there anyone who might be able to help me iterate over a fix or maybe a new maintainer of the PDB parsing?

Jul 30 2019, 12:53 PM · Restricted Project

Jul 29 2019

shafik added a comment to D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

@stella.stamenova this could potentially break the windows build, could you please verify before I land this change. Thank you in advance!

Jul 29 2019, 3:56 PM · Restricted Project
shafik updated the diff for D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

Simplifying Makefile even more

Jul 29 2019, 3:51 PM · Restricted Project
shafik added a reviewer for D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl: stella.stamenova.
Jul 29 2019, 2:47 PM · Restricted Project
shafik updated the diff for D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

Addressing comments:

  • Simplifying Makefile
  • Adding comments to the test
Jul 29 2019, 2:42 PM · Restricted Project
shafik added a comment to D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.

@aprantl @clayborg Thank you for the comments, I think I have addressed your concerns.

Jul 29 2019, 2:42 PM · Restricted Project
shafik created D65414: Fix ClangASTContext::CreateParameterDeclaration to not call addDecl.
Jul 29 2019, 1:09 PM · Restricted Project

Jul 25 2019

shafik added a comment to D59692: [ASTImporter] Fix name conflict handling with different strategies.

First round of review.

Jul 25 2019, 3:33 PM · Restricted Project, Restricted Project

Jul 24 2019

shafik added a comment to D64917: Add offsetof support to expression evaluator..

Thanks for fixing this one!

Jul 24 2019, 3:58 PM · Restricted Project
shafik accepted D44100: [ASTImporter] Reorder fields after structure import is finished.

LGTM

Jul 24 2019, 2:37 PM · Restricted Project, Restricted Project

Jul 22 2019

shafik added a comment to D64995: [lldb] Fix crash when tab-completing in multi-line expr.

Also once we get one test going then it should be easy to add coverage for all sorts of scenarios. Who knows maybe we will find more bugs.

Jul 22 2019, 3:46 PM · Restricted Project, Restricted Project

Jul 17 2019

shafik committed rGa0858e2f20c8: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack… (authored by shafik).
Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack…
Jul 17 2019, 1:22 PM
shafik committed rL366365: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack….
Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack…
Jul 17 2019, 1:22 PM
shafik closed D64777: Fix CreateFunctionTemplateSpecialization to prevent dangling poiner to stack memory.
Jul 17 2019, 1:22 PM · Restricted Project
shafik added a comment to D64858: [lldb] Make log for ClangModulesDeclVendor's compiler flag less verbose.

Is it worth it to write a test that verifies the output? Otherwise LGTM.

Jul 17 2019, 9:28 AM · Restricted Project, Restricted Project