Page MenuHomePhabricator

zahiraam (Zahira Ammarguellat)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 14 2017, 11:28 AM (105 w, 4 d)

Recent Activity

Tue, Apr 2

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

There are still a few things I haven't addressed yet. Mostly because not sure there is another solution like getting rid of the map from StringRef to expr. The other issue is not adding new kind to ParsedAttr. There may be another way of doing it, but didn't look at it yet.
Meanwhile can you please let me know if you are happy with the current status of the implementation.

Tue, Apr 2, 5:22 AM
zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Tue, Apr 2, 5:19 AM

Mar 19 2019

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Mar 19 2019, 12:56 PM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

It would be nice to have a review for this year old (updated) patch. Thanks.

Mar 19 2019, 12:56 PM

Mar 18 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

LGTM!

Mar 18 2019, 6:49 AM

Mar 11 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Mar 11 2019, 10:55 AM
zahiraam added inline comments to D45978: dllexport const variables must have external linkage..
Mar 11 2019, 10:53 AM

Mar 8 2019

zahiraam added inline comments to D45978: dllexport const variables must have external linkage..
Mar 8 2019, 4:36 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Mar 8 2019, 4:36 AM

Mar 7 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Mar 7 2019, 12:33 AM

Feb 25 2019

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
In D41950#1404899, @rnk wrote:

lgtm

Feb 25 2019, 9:41 AM · Restricted Project

Feb 23 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

Let's see if I have included every thing mentioned. Thanks.

Feb 23 2019, 6:40 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 23 2019, 6:40 AM

Feb 20 2019

zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 20 2019, 7:14 AM · Restricted Project
zahiraam added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 20 2019, 7:14 AM · Restricted Project

Feb 14 2019

zahiraam added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 14 2019, 6:54 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 14 2019, 6:54 AM · Restricted Project
zahiraam added inline comments to D45978: dllexport const variables must have external linkage..
Feb 14 2019, 2:24 AM

Feb 13 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 13 2019, 6:36 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..
Feb 13 2019, 6:36 AM

Feb 12 2019

zahiraam updated subscribers of D45978: dllexport const variables must have external linkage..
Feb 12 2019, 7:10 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 12 2019, 7:10 AM

Feb 11 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 11 2019, 9:14 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..

It looks like the patch got mucked up somehow, I only see three testing files in the patch now?

Feb 11 2019, 9:14 AM

Feb 8 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 8 2019, 12:37 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..

Can you add tests for C mode as well, as it seems the behavior differs there.

Feb 8 2019, 12:37 AM

Feb 6 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

That seems like a reasonable place to try, to me.

Feb 6 2019, 6:33 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 6 2019, 6:33 AM

Feb 3 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..
In D45978#1379901, @rnk wrote:

I'm still not sure this is the best place to make this change, but the functionality is important. There are still unaddressed comments (no need to check MSVCCompatibility, formatting), and I think once those are fixed we can land this.

Feb 3 2019, 3:17 AM

Jan 30 2019

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..

Feedback please. Thanks.

Jan 30 2019, 5:50 AM · Restricted Project

Jan 26 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

Please advise.
Thanks.

Jan 26 2019, 2:07 AM

Nov 24 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..

Feedback on this please. Thanks.

Nov 24 2018, 3:39 AM · Restricted Project

May 14 2018

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
May 14 2018, 10:50 AM

May 1 2018

zahiraam added a comment to D46226: Oenmp task with by reference array.

Is this better? Thanks.

May 1 2018, 8:09 AM
zahiraam updated the diff for D46226: Oenmp task with by reference array.
May 1 2018, 8:09 AM

Apr 28 2018

zahiraam added a comment to D46226: Oenmp task with by reference array.
Apr 28 2018, 9:08 AM
zahiraam created D46226: Oenmp task with by reference array.
Apr 28 2018, 9:08 AM

Apr 25 2018

zahiraam added a comment to D45978: dllexport const variables must have external linkage..
Apr 25 2018, 11:05 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..
In D45978#1077051, @rnk wrote:

This shouldn't be conditional under -fms-compatibility, it's part of dllexport, which is already under -fms-extensions / -fdeclspec-extensions.

I don't think this is the right place to do this. Can it be done as part of processing the dllexport attribute or as part of the main storage class calculation?

Apr 25 2018, 11:05 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Apr 25 2018, 11:04 AM

Apr 23 2018

zahiraam created D45978: dllexport const variables must have external linkage..
Apr 23 2018, 11:48 AM

Apr 20 2018

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

Richard,
Please let me know if I have answered to all the issues you raised. Thanks.

Apr 20 2018, 8:33 AM
zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 20 2018, 8:33 AM
zahiraam added inline comments to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 20 2018, 8:30 AM

Apr 15 2018

zahiraam added inline comments to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 15 2018, 9:06 AM

Apr 14 2018

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 14 2018, 8:25 AM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

We should really, really avoid making this sort of change without first trying to desugar uuidof into a reference to a variable. That would solve a ton of problems, problems like this one.

Apr 14 2018, 8:20 AM

Apr 13 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 13 2018, 8:14 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 13 2018, 8:14 AM · Restricted Project

Apr 11 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 11 2018, 9:45 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 11 2018, 9:45 AM · Restricted Project

Apr 9 2018

zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 9 2018, 4:12 AM · Restricted Project

Mar 20 2018

zahiraam accepted rL327754: [MS] Fix bug in r327732 with devirtualized complete destructor calls.
Mar 20 2018, 11:50 AM

Mar 19 2018

zahiraam raised a concern with rL327754: [MS] Fix bug in r327732 with devirtualized complete destructor calls.

This is the LLVM I get. The above test case is failing:

Mar 19 2018, 3:19 PM
zahiraam added a comment to rL327754: [MS] Fix bug in r327732 with devirtualized complete destructor calls.

I think this breaks CodeGenCXX/dllimport-base.cpp and devirtualize-ms-dtor.cpp?

Mar 19 2018, 6:10 AM

Mar 16 2018

zahiraam accepted D44505: [MS] Always use base dtors in place of complete/vbase dtors when possible.
Mar 16 2018, 12:08 PM
zahiraam added a comment to D44505: [MS] Always use base dtors in place of complete/vbase dtors when possible.

LGTM 2. It fixes PR32990.

Mar 16 2018, 12:08 PM

Mar 9 2018

zahiraam added a comment to D39063: Fix for PR32990..
Mar 9 2018, 1:25 PM
zahiraam updated the diff for D39063: Fix for PR32990..
Mar 9 2018, 1:25 PM

Mar 8 2018

zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..

Attached the test case.

Mar 8 2018, 11:59 AM
zahiraam updated the diff for D38803: Suppress generation of a deleting destructor for a dllimport record..

Test case to add.

Mar 8 2018, 11:59 AM
zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..
In D38803#1030905, @rnk wrote:

Can you check the base revision you used to upload the patch? I think there should just be new test case code now.

Yes. Can you please commit the test case?

I believe he's saying you should rebase, since an additional testcase has already been added.

Oops! I thought I had done it already. Sorry. On it.

Mar 8 2018, 7:11 AM
zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..
In D38803#1030905, @rnk wrote:

Can you check the base revision you used to upload the patch? I think there should just be new test case code now.

Mar 8 2018, 6:24 AM

Mar 2 2018

zahiraam added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Mar 2 2018, 6:27 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..

Hopefully I have responded to the issues you have raised.
I have added the test case you have submitted, to make sure that an expression is generated.
Some of the error messages in the lit test have to be updated (they do match MSVC error messages).

Mar 2 2018, 6:26 AM · Restricted Project

Feb 28 2018

zahiraam added reviewers for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword.: hans, erichkeane.
Feb 28 2018, 6:09 AM · Restricted Project

Feb 26 2018

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

Here's my thinking: the __uuidof expression literally declares a variable called _GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3 of type __s_GUID which is why it behaves the way it does: https://godbolt.org/g/74FY7U

This is an implementation detail leaking, though. no? Note that that is a reserved name.

I don't think it is reasonable to invent new semantics which are different from the MSVC ones because we find the MSVC ones inelegant.

I mostly agree, but my point is that this is *not* the MSVC semantics, it's merely an implementation detail that non-conforming code happens to be able to observe. Suppose that type_info objects were similarly accessible in MSVC by guessing their mangled names. Would you be arguing that we should inject variables for those too? (And note that it is *nearly* true that type_info objects work that way: https://godbolt.org/g/zByFFg -- but the parser gets confused somehow when you reference them.) The only difference I can see between these cases is that the reserved name used for the GUID case happens to not contain any ?s and @s, so happens to be utterable as an identifier.

We should not attempt to be compatible with the cases where MSVC's implementation details happen to leak into user-visible semantics.

What is the relative upside to a new kind of Decl? Better AST fidelity?

Yes, exactly. The same reason we don't desguar other things any more than we have to.

Feb 26 2018, 1:03 PM
zahiraam added a comment to D42968: Fix for PR32992. Static const classes not exported..

I think you only uploaded the diff between this and the previous version. But the S.MarkVariableReferenced version LGTM. Do you want me to commit it for you?

Feb 26 2018, 6:28 AM

Feb 23 2018

zahiraam added inline comments to D42968: Fix for PR32992. Static const classes not exported..
Feb 23 2018, 10:50 AM
zahiraam updated the diff for D42968: Fix for PR32992. Static const classes not exported..
Feb 23 2018, 10:49 AM

Feb 22 2018

zahiraam added a comment to D42968: Fix for PR32992. Static const classes not exported..

Added a test case to check the suppression of the warning.

Feb 22 2018, 2:15 PM
zahiraam updated the diff for D42968: Fix for PR32992. Static const classes not exported..
Feb 22 2018, 2:14 PM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

We should really, really avoid making this sort of change without first trying to desugar uuidof into a reference to a variable. That would solve a ton of problems, problems like this one.

Feb 22 2018, 12:49 PM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

Adding test case.

Feb 22 2018, 12:25 PM
zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Feb 22 2018, 12:25 PM

Feb 21 2018

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Feb 21 2018, 7:53 AM
zahiraam created D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Feb 21 2018, 7:53 AM

Feb 20 2018

zahiraam added a comment to D42968: Fix for PR32992. Static const classes not exported..
Feb 20 2018, 1:34 PM
zahiraam updated the diff for D42968: Fix for PR32992. Static const classes not exported..

Fixing the assert in the build.

Feb 20 2018, 1:34 PM

Feb 19 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 19 2018, 9:03 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 19 2018, 9:03 AM · Restricted Project

Feb 17 2018

zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..

Rebasing on rL315656 actually fixes the problem. No deleting destructor is generated. So adding the test case only to insure that is enough. The code of this release is already committed.

Feb 17 2018, 10:06 AM
zahiraam updated the diff for D38803: Suppress generation of a deleting destructor for a dllimport record..
Feb 17 2018, 10:05 AM

Feb 16 2018

zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..

Added my patch to https://reviews.llvm.org/rL315656 .

Feb 16 2018, 8:05 AM
zahiraam updated the diff for D38803: Suppress generation of a deleting destructor for a dllimport record..
Feb 16 2018, 8:05 AM
zahiraam abandoned D40551: Member access to incomplete type from dllimport.

See https://reviews.llvm.org/D40621

Feb 16 2018, 7:23 AM
zahiraam added a comment to rL315656: [MS] Don't bail on replacing dllimport vbase dtors with base dtors.

This code and the (updated) test case are all included in the review here:
https://reviews.llvm.org/D39063

Feb 16 2018, 7:02 AM

Feb 13 2018

zahiraam added a comment to D39610: instantiation of uuid with dllexport..

I would appreciate feedback on this please! May be adding another reviewer if you guys don't have time? Thanks.

Feb 13 2018, 6:26 AM

Feb 12 2018

zahiraam added a comment to D42968: Fix for PR32992. Static const classes not exported..

Looks good to me. Do you have commit access or would you like me to commit it for you?

Feb 12 2018, 6:03 AM

Feb 9 2018

zahiraam updated the diff for D42968: Fix for PR32992. Static const classes not exported..
Feb 9 2018, 10:29 AM
zahiraam added a comment to D42968: Fix for PR32992. Static const classes not exported..
Feb 9 2018, 10:29 AM

Feb 8 2018

zahiraam added inline comments to D42968: Fix for PR32992. Static const classes not exported..
Feb 8 2018, 3:01 PM
zahiraam updated the diff for D42968: Fix for PR32992. Static const classes not exported..
Feb 8 2018, 3:01 PM

Feb 6 2018

zahiraam created D42968: Fix for PR32992. Static const classes not exported..
Feb 6 2018, 7:33 AM

Jan 23 2018

zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Jan 23 2018, 6:38 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Jan 23 2018, 6:38 AM · Restricted Project

Jan 18 2018

zahiraam added a comment to D39063: Fix for PR32990..
In D39063#979580, @rnk wrote:

Sorry, I haven't had time to put together a reproduction over the holidays. Looks like it affects STL classes.

Jan 18 2018, 6:10 AM

Jan 11 2018

zahiraam added a comment to D39063: Fix for PR32990..

Reid,
Can you please suggest a test case to fix the build? I can't seem to find scenario to break it. Thanks.

Jan 11 2018, 8:12 AM
zahiraam added a comment to D39610: instantiation of uuid with dllexport..

Can you please let me know if this is a solution that you would consider? Thanks.

Jan 11 2018, 7:39 AM
zahiraam created D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Jan 11 2018, 7:36 AM · Restricted Project