Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

Thanks a lot Akira! FWIW, it seems that the FIXME here might be related https://github.com/llvm-mirror/clang/blob/master/include/clang/AST/DeclCXX.h#L1484 On Mon, Mar 12, 2018 at 6:03 PM Akira Hatanaka wrote: > I’m not sure if this a bug r327206 introduced or an existing

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

I’m not sure if this a bug r327206 introduced or an existing bug in ASTImporter as there are other bits in RecordDecl that are not copied, but I think I should revert the patch for now. > On Mar 12, 2018, at 9:57 AM, Eric Liu wrote: > > I think it's likely as our tests only

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

I think it's likely as our tests only fail with module enabled (without module, ASTImporter isn't really used). On Mon, Mar 12, 2018 at 5:51 PM Akira Hatanaka wrote: > The patch I committed moved CXXRecordDecl::CanPassInRegisters to > RecordDecl. It looks like

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

The patch I committed moved CXXRecordDecl::CanPassInRegisters to RecordDecl. It looks like ASTImporter::ImportDefinition no longer copies the bit for CanPassInRegisters after that change. I’m not sure that is what’s causing tests to fail. > On Mar 12, 2018, at 9:29 AM, Eric Liu

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

I have been trying to reduce a reproducer for this but haven't gotten any luck yet. The error happens in conversion between different version of STL containers and is a bit hard to reduce. I'll keep trying to create a reproducer. Could you please also take a quick look to see if

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

Do you have a reproducer? > On Mar 12, 2018, at 9:07 AM, Eric Liu wrote: > > I think there is a bug in the ASTImporter/Reader/Writer, but I'm not sure > what's the right way to fix it. I'll revert this commit for now to unblock > integration. Let me know if you need more

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

I think there is a bug in the ASTImporter/Reader/Writer, but I'm not sure what's the right way to fix it. I'll revert this commit for now to unblock integration. Let me know if you need more information from us. Regards, Eric On Mon, Mar 12, 2018 at 4:51 PM Eric Liu wrote: >

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

The tests only failed with module enabled. FWIW, I think the change in ASTImporter (https://reviews.llvm.org/rL327206#change-1q8vFFjJ6Cqk) needs additional changes to make imports work for RecordDecl. On Mon, Mar 12, 2018 at 3:56 PM Eric Liu wrote: > Hi Akira, > > It seems

Re: r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

Hi Akira, It seems that this commit also changes behavior for compiling C++ code as we are seeing test failures caused by this change in our internal tests. I'm still trying to reduce a reproducer for the failure. In the meantime, could you please double check if this affects C++? Thanks, Eric

r327206 - [ObjC] Allow declaring __weak pointer fields in C structs in ARC.

Author: ahatanak Date: Fri Mar 9 22:36:08 2018 New Revision: 327206 URL: http://llvm.org/viewvc/llvm-project?rev=327206=rev Log: [ObjC] Allow declaring __weak pointer fields in C structs in ARC. This patch uses the infrastructure added in r326307 for enabling non-trivial fields to be declared