This revision was automatically updated to reflect the committed changes.
Closed by commit rL270808: [ObjC] Remove _Atomic from return type and parameter
type of (authored by ahatanak).
Changed prior to commit:
http://reviews.llvm.org/D20407?vs=58544&id=58549#toc
Repository:
rL LLVM
http://
ahatanak updated this revision to Diff 58544.
ahatanak added a comment.
Thanks for the review. I've removed parameter ASTContext that was unused.
http://reviews.llvm.org/D20407
Files:
include/clang/AST/Type.h
lib/AST/Type.cpp
lib/CodeGen/CGObjC.cpp
lib/Sema/SemaObjCProperty.cpp
test/C
rjmccall added a comment.
One minor improvement to the API and LGTM.
Comment at: include/clang/AST/Type.h:1084
@@ +1083,3 @@
+ /// Remove all qualifiers including _Atomic.
+ QualType getAtomicUnqualifiedType(const ASTContext &ctx) const;
+
This doesn't need an
rjmccall added a comment.
In http://reviews.llvm.org/D20407#440050, @ahatanak wrote:
> In http://reviews.llvm.org/D20407#439951, @rjmccall wrote:
>
> > The C standard is poorly-written in this area, but I think it would be
> > reasonable for CheckFunctionReturnType to just silently remove _Atomi
ahatanak added a comment.
In http://reviews.llvm.org/D20407#439951, @rjmccall wrote:
> The C standard is poorly-written in this area, but I think it would be
> reasonable for CheckFunctionReturnType to just silently remove _Atomic. (You
> will not be able to just re-use your new method there;
ahatanak updated this revision to Diff 58512.
ahatanak added a comment.
Rename variables.
http://reviews.llvm.org/D20407
Files:
include/clang/AST/Type.h
lib/AST/Type.cpp
lib/CodeGen/CGObjC.cpp
lib/Sema/SemaObjCProperty.cpp
test/CodeGenObjC/property-atomic-bool.m
test/SemaObjC/proper
rjmccall added a comment.
In http://reviews.llvm.org/D20407#439887, @ahatanak wrote:
> I reverted the changes I made in SemaDeclObjC.cpp as they weren't needed to
> pass the regression tests I added. clang still asserts when it compiles an
> objective-c method returning _Atomic and those change
rjmccall added inline comments.
Comment at: lib/AST/Type.cpp:1282
@@ -1277,1 +1281,3 @@
+}
+
Optional> Type::getObjCSubstitutions(
ahatanak wrote:
> I added getTypePtr() because the code didn't compile.
Sure.
Comment at: lib/CodeGen/CGObjC.cpp:9
ahatanak marked 2 inline comments as done.
ahatanak added a comment.
I reverted the changes I made in SemaDeclObjC.cpp as they weren't needed to
pass the regression tests I added. clang still asserts when it compiles an
objective-c method returning _Atomic and those changes will become necessary
ahatanak marked 7 inline comments as done.
Comment at: lib/AST/Type.cpp:1282
@@ -1277,1 +1281,3 @@
+}
+
Optional> Type::getObjCSubstitutions(
I added getTypePtr() because the code didn't compile.
Comment at: lib/CodeGen/CGObjC.cpp:901
@@ -903,1
ahatanak updated this revision to Diff 58495.
ahatanak added a comment.
Address John's review comments.
http://reviews.llvm.org/D20407
Files:
include/clang/AST/Type.h
lib/AST/Type.cpp
lib/CodeGen/CGObjC.cpp
lib/Sema/SemaObjCProperty.cpp
test/CodeGenObjC/property-atomic-bool.m
test/S
rjmccall added inline comments.
Comment at: include/clang/AST/Type.h:1084
@@ +1083,3 @@
+ /// Strip typedefs and atomic from the given type.
+ QualType getDesugaredAtomicValueType(const ASTContext &ctx) const;
+
Please name this getAtomicUnqualifiedType() and ha
ahatanak updated this revision to Diff 58144.
ahatanak added a comment.
Rewrote the patch based on John's review comment.
Remove typedefs and _Atomic from the return and parameter types of getters and
setters of objective-c properties.
http://reviews.llvm.org/D20407
Files:
include/clang/AST
rjmccall added a comment.
Top-level attributes shouldn't really be affecting the ABI or code-generation
in any way, so desugaring should be fine.
http://reviews.llvm.org/D20407
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.ll
ahatanak added a comment.
I've managed to remove _Atomic from the types of the return and parameter of
getter and setter functions in my new patch.
I'm not sure how I should handle typedefs though. If I had the following
typedef,
typedef _Atomic(_Bool) AtomicBool,
would it be OK to desugar th
ahatanak added a comment.
In http://reviews.llvm.org/D20407#433915, @rjmccall wrote:
> _Atomic is functionally a type qualifier and should be removed in Sema when
> computing the result type of the getter and the parameter type of the setter.
> That is, if the user declares a property of type
rjmccall added a comment.
_Atomic is functionally a type qualifier and should be removed in Sema when
computing the result type of the getter and the parameter type of the setter.
That is, if the user declares a property of type _Atomic(_Bool), we should
pretend that the property has type _Boo
ahatanak added a comment.
Also, it seems that there are bugs in the way clang handles functions whose
return types are atomic. Clang asserts when compiling the following test case:
$ cat t1.c
_Atomic _Bool b1;
_Atomic _Bool foo1() {
return b1;
}
$ clang -std=c11 -o - -S t1.c -emit
ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a subscriber: cfe-commits.
This patch fixes an assert that fires when there is a property that has
attribute nonatomic and type _Atomic(_Bool). The assert fires when an i1 value
is bitcast to i8 (which is the typ
19 matches
Mail list logo
