Re: Change Request

[Aaron Ballman via cfe-commits]

On Thu, Mar 10, 2016 at 11:38 AM, Wes Witt via cfe-commits
 wrote:
> I would like to submit the attached changes for your approval.

Thank you for the patch!

This:

+  let Subjects = SubjectList<[Function, Var, CXXRecord,
ObjCInterface], WarnDiag,
+ "ExpectedVariableOrFunction">;

should be using ExpectedFunctionVariableOrClass. It's not completely
correct since you're adding Objective-C interfaces, but it's a bit
better than dropping the class part.

Also, patch submissions should include test cases exercising the
differences in the patch, so you should add some tests.

Welcome!

~Aaron
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Change Request

[Jonathan Roelofs via cfe-commits]

On 3/10/16 9:38 AM, Wes Witt via cfe-commits wrote:

I would like to submit the attached changes for your approval.


Gentle etiquette suggestion: please title your emails with something 
more descriptive. In this case, something like: "[PATCH] Windows support 
for ObjC codegen". And then in the body of the email, write a little 
blurb saying how the changes achieve that goal, and why they're necessary.




My name is Wes Witt and I’m a software engineer at Microsoft. The diff
represent some small changes that we’ve made in order to use Clang to
compile Objective C on Windows.

The compiler is used for the open source WinObjC project
(https://github.com/Microsoft/WinObjC).


While you're here changing the signature anyway, please change Name to 
be a `StringRef` instead of a `const std::string&`:


   virtual llvm::Value *GetClassNamed(CodeGenFunction ,
-  const std::string , bool isWeak);
+  const std::string , bool isWeak, bool isDLLImport);


This:

+bool isDLLImport = false;
+if (OID->hasAttr()) {
+isDLLImport = true;
+}
+return GetClassNamed(CGF, OID->getNameAsString(), 
OID->isWeakImported(), isDLLImport);


should be written as:

return GetClassNamed(CGF, OID->getNameAsString(), OID->isWeakImported(), 
OID->hasAttr());



This:

+  if (isDLLImport) 
ClassSymbol->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);


should be broken into two lines.

-  return GetClassNamed(CGF, "NSAutoreleasePool", false);
+  return GetClassNamed(CGF, "NSAutoreleasePool", false, true);

I think the last parameter there should be an "is this a windows target" 
check, rather than an unconditional 'true'.



You'll also need to write testcases to verify the changed behavior. Have 
a look in clang/test/CodeGenObjC for examples of how to do it.




Thank you for your consideration.


Welcome to the community, and happy hacking!


Jon



-Wes Witt

w...@microsoft.com 



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



--
Jon Roelofs
jonat...@codesourcery.com
CodeSourcery / Mentor Embedded
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Change Request

[Wes Witt via cfe-commits]

I would like to submit the attached changes for your approval.

My name is Wes Witt and I'm a software engineer at Microsoft. The diff 
represent some small changes that we've made in order to use Clang to compile 
Objective C on Windows.

The compiler is used for the open source WinObjC project 
(https://github.com/Microsoft/WinObjC).

Thank you for your consideration.

-Wes Witt
w...@microsoft.com



clang-changes.diff
Description: clang-changes.diff
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits