[PATCH] D46112: Allow _Atomic to be specified on incomplete types
jfb added a comment.
In https://reviews.llvm.org/D46112#1113557, @aaron.ballman wrote:
> In https://reviews.llvm.org/D46112#1098243, @rsmith wrote:
>
> > In https://reviews.llvm.org/D46112#1096893, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D46112#1091981, @efriedma wrote:
> > >
> > > > Also needs some test coverage for atomic operations which aren't calls,
> > > > like "typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s =
> > > > *s2; };".
> > >
> > >
> > > Thank you for pointing this out -- that uncovered an issue where we were
> > > not properly diagnosing the types as being incomplete. I've added a new
> > > test case and rolled the contents of Sema\atomic-type.cpp (which I added
> > > in an earlier patch) into SemaCXX\atomic-type.cpp (which already existed
> > > and I missed it).
> >
> >
> > I don't think this has sufficiently addressed the comment. Specifically,
> > for a case like:
> >
> > struct NotTriviallyCopyable { NotTriviallyCopyable(const
> > NotTriviallyCopyable&); };
> > void f(_Atomic NotTriviallyCopyable *p) { *p = *p; }
> >
> >
> > ... we should reject the assignment, because it would perform an atomic
> > operation on a non-trivially-copyable type. It would probably suffice to
> > check the places where we form an `AtomicToNonAtomic` or
> > `NonAtomicToAtomic` conversion.
> >
> > In passing, I noticed that this crashes Clang (because we fail to create an
> > lvalue-to-rvalue conversion for `x`):
> >
> > struct X {};
> > void f(_Atomic X *p, X x) { *p = x; }
> >
> >
> > This will likely get in the way of your test cases unless you fix it :)
>
>
> It only gets in the way for C++ whereas my primary concern for this patch is
> C. I tried spending a few hours on this today and got nowhere -- we have a
> lot of FIXME comments surrounding atomic type qualifiers in C++. I've run out
> of time to be able to work on this patch and may have to abandon it. I'd hate
> to lose the forward progress already made, so I'm wondering if the C bits are
> sufficiently baked that even more FIXMEs around atomics in C++ would suffice?
FYI I ran into this here: https://reviews.llvm.org/D49458
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
efriedma added a comment.
If you don't want to spend too much time on C++, fine; could you add a short
Objective-C test instead to make sure the trivially-copyable checks are working?
What are the changes to Sema::RequireCompleteTypeImpl supposed to do?
Comment at: test/Sema/atomic-type.c:30
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to
atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void)
*' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of
incomplete type 'void'}}
This error message is terrible; yes, technically 'void' isn't trivially
copyable, but that isn't really helpful.
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman added a comment.
In https://reviews.llvm.org/D46112#1098243, @rsmith wrote:
> In https://reviews.llvm.org/D46112#1096893, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D46112#1091981, @efriedma wrote:
> >
> > > Also needs some test coverage for atomic operations which aren't calls,
> > > like "typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2;
> > > };".
> >
> >
> > Thank you for pointing this out -- that uncovered an issue where we were
> > not properly diagnosing the types as being incomplete. I've added a new
> > test case and rolled the contents of Sema\atomic-type.cpp (which I added in
> > an earlier patch) into SemaCXX\atomic-type.cpp (which already existed and I
> > missed it).
>
>
> I don't think this has sufficiently addressed the comment. Specifically, for
> a case like:
>
> struct NotTriviallyCopyable { NotTriviallyCopyable(const
> NotTriviallyCopyable&); };
> void f(_Atomic NotTriviallyCopyable *p) { *p = *p; }
>
>
> ... we should reject the assignment, because it would perform an atomic
> operation on a non-trivially-copyable type. It would probably suffice to
> check the places where we form an `AtomicToNonAtomic` or `NonAtomicToAtomic`
> conversion.
>
> In passing, I noticed that this crashes Clang (because we fail to create an
> lvalue-to-rvalue conversion for `x`):
>
> struct X {};
> void f(_Atomic X *p, X x) { *p = x; }
>
>
> This will likely get in the way of your test cases unless you fix it :)
It only gets in the way for C++ whereas my primary concern for this patch is C.
I tried spending a few hours on this today and got nowhere -- we have a lot of
FIXME comments surrounding atomic type qualifiers in C++. I've run out of time
to be able to work on this patch and may have to abandon it. I'd hate to lose
the forward progress already made, so I'm wondering if the C bits are
sufficiently baked that even more FIXMEs around atomics in C++ would suffice?
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman updated this revision to Diff 148753.
aaron.ballman marked 4 inline comments as done.
aaron.ballman added a comment.
Corrected some of the review comments.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaType.cpp
test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
test/CodeGen/c11atomics.c
test/Sema/atomic-type.c
test/SemaCXX/atomic-type.cpp
test/SemaObjC/arc.m
Index: test/SemaObjC/arc.m
===
--- test/SemaObjC/arc.m
+++ test/SemaObjC/arc.m
@@ -410,7 +410,7 @@
[v test16_6: 0];
}
-@class Test17; // expected-note 2{{forward declaration of class here}}
+@class Test17; // expected-note 3{{forward declaration of class here}}
@protocol Test17p
- (void) test17;
+ (void) test17;
Index: test/SemaCXX/atomic-type.cpp
===
--- test/SemaCXX/atomic-type.cpp
+++ test/SemaCXX/atomic-type.cpp
@@ -94,3 +94,15 @@
bool PR21836(_Atomic(int) *x) {
return *x;
}
+
+struct T;
+
+void f(_Atomic T *t);
+
+struct T { int a, b; };
+
+void f(_Atomic T *t) {
+ T t2;
+ (void)__atomic_load(t, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(T) *' invalid)}}
+ (void)__c11_atomic_load(t, 5);
+}
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,23 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2; // expected-error {{tentative definition has type '_Atomic(struct ErrorS)' that is never completed}} \
+ // expected-note {{forward declaration of 'struct ErrorS'}}
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+ _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
+
+typedef struct S S; // expected-note {{forward declaration of 'struct S'}}
+void test_atomic_incomplete_struct_assignment(_Atomic S *s, _Atomic S *s2) {
+ *s = *s2; // expected-error {{incomplete type '_Atomic(S)' is not assignable}}
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
// we have to generate an atomic add, which returns the old value, and then a
// non-atomic add.
// CHECK: atomicrmw add i32* @i, i32 1 seq_cst
- // CHECK: add i32
+ // CHECK: add i32
++i;
// CHECK: atomicrmw add i64* @l, i64 1 seq_cst
// CHECK: add i64
@@ -475,6 +475,25 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+int test_atomic_void_ptr(_Atomic void *addr) {
+ // CHECK-LABEL: @test_atomic_void_ptr(
+ // CHECK: [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+ // CHECK: [[ATOMIC_RES:%.*]] = alloca i32, align 4
+ // CHECK: store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+ // CHECK: call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+ // CHECK: [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+ // CHECK: [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+ // CHECK: [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+ // CHECK: store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+ // CHECK: [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+ // CHECK: ret i32 [[RES]]
+ (void)__atomic_is_lock_free(1, addr);
+ return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
struct Empty {};
struct Empty test_empty_struct_load(_Atomic(struct Empty)*
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
rsmith added a comment.
In https://reviews.llvm.org/D46112#1096893, @aaron.ballman wrote:
> In https://reviews.llvm.org/D46112#1091981, @efriedma wrote:
>
> > Also needs some test coverage for atomic operations which aren't calls,
> > like "typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2;
> > };".
>
>
> Thank you for pointing this out -- that uncovered an issue where we were not
> properly diagnosing the types as being incomplete. I've added a new test case
> and rolled the contents of Sema\atomic-type.cpp (which I added in an earlier
> patch) into SemaCXX\atomic-type.cpp (which already existed and I missed it).
I don't think this has sufficiently addressed the comment. Specifically, for a
case like:
struct NotTriviallyCopyable { NotTriviallyCopyable(const
NotTriviallyCopyable&); };
void f(_Atomic NotTriviallyCopyable *p) { *p = *p; }
... we should reject the assignment, because it would perform an atomic
operation on a non-trivially-copyable type. It would probably suffice to check
the places where we form an `AtomicToNonAtomic` or `NonAtomicToAtomic`
conversion.
In passing, I noticed that this crashes Clang (because we fail to create an
lvalue-to-rvalue conversion for `x`):
struct X {};
void f(_Atomic X *p, X x) { *p = x; }
This will likely get in the way of your test cases unless you fix it :)
Comment at: lib/Sema/SemaChecking.cpp:3202
- if (!IsC11 && !AtomTy.isTriviallyCopyableType(Context) &&
- !AtomTy->isScalarType()) {
-// For GNU atomics, require a trivially-copyable type. This is not part of
-// the GNU atomics specification, but we enforce it for sanity.
+ if (!ValType.isTriviallyCopyableType(Context) && !ValType->isScalarType()) {
+// Always require a trivially-copyable type. This is not part of the GNU
The (pre-existing) `isScalarType()` check here looks wrong to me. If we had a
non-trivially-copyable scalar type (which I think do not currently exist in our
type system), I think we should reject it here.
Comment at: lib/Sema/SemaType.cpp:7662
// try to instantiate it.
- QualType MaybeTemplate = T;
- while (const ConstantArrayType *Array
- = Context.getAsConstantArrayType(MaybeTemplate))
-MaybeTemplate = Array->getElementType();
- if (const RecordType *Record = MaybeTemplate->getAs()) {
+ if (auto *RD = dyn_cast_or_null(Def)) {
bool Instantiated = false;
May as well cast directly to `CXXRecordDecl` here; none of the code controlled
by this `if` does anything in C.
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman marked 8 inline comments as done. aaron.ballman added inline comments. Comment at: lib/Sema/SemaType.cpp:7604-7608 + // When instantiating a class template and reaching an atomic type, the check + // for type completeness should occur on the underlying type and not the + // atomic type itself (which is always incomplete). + if (inTemplateInstantiation() && T->isAtomicType()) +T = cast(T)->getValueType(); rsmith wrote: > This function is duplicating the work of finding the type to complete, which > was already done by `isIncompleteType`. Rather than unwrapping `T` here... Thank you for the explanation! https://reviews.llvm.org/D46112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman updated this revision to Diff 146594.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
Updated based on review comments.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaType.cpp
test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
test/CodeGen/c11atomics.c
test/Sema/atomic-type.c
test/SemaCXX/atomic-type.cpp
test/SemaObjC/arc.m
Index: test/SemaObjC/arc.m
===
--- test/SemaObjC/arc.m
+++ test/SemaObjC/arc.m
@@ -410,7 +410,7 @@
[v test16_6: 0];
}
-@class Test17; // expected-note 2{{forward declaration of class here}}
+@class Test17; // expected-note 3{{forward declaration of class here}}
@protocol Test17p
- (void) test17;
+ (void) test17;
Index: test/SemaCXX/atomic-type.cpp
===
--- test/SemaCXX/atomic-type.cpp
+++ test/SemaCXX/atomic-type.cpp
@@ -94,3 +94,15 @@
bool PR21836(_Atomic(int) *x) {
return *x;
}
+
+struct T;
+
+void f(_Atomic T *t);
+
+struct T { int a, b; };
+
+void f(_Atomic T *t) {
+ T t2;
+ (void)__atomic_load(t, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(T) *' invalid)}}
+ (void)__c11_atomic_load(t, 5);
+}
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,23 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2; // expected-error {{tentative definition has type '_Atomic(struct ErrorS)' that is never completed}} \
+ // expected-note {{forward declaration of 'struct ErrorS'}}
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+ _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
+
+typedef struct S S; // expected-note {{forward declaration of 'struct S'}}
+void test_atomic_incomplete_struct_assignment(_Atomic S *s, _Atomic S *s2) {
+ *s = *s2; // expected-error {{incomplete type '_Atomic(S)' is not assignable}}
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
// we have to generate an atomic add, which returns the old value, and then a
// non-atomic add.
// CHECK: atomicrmw add i32* @i, i32 1 seq_cst
- // CHECK: add i32
+ // CHECK: add i32
++i;
// CHECK: atomicrmw add i64* @l, i64 1 seq_cst
// CHECK: add i64
@@ -475,6 +475,25 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+int test_atomic_void_ptr(_Atomic void *addr) {
+ // CHECK-LABEL: @test_atomic_void_ptr(
+ // CHECK: [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+ // CHECK: [[ATOMIC_RES:%.*]] = alloca i32, align 4
+ // CHECK: store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+ // CHECK: call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+ // CHECK: [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+ // CHECK: [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+ // CHECK: [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+ // CHECK: store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+ // CHECK: [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+ // CHECK: ret i32 [[RES]]
+ (void)__atomic_is_lock_free(1, addr);
+ return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
struct Empty {};
struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty)
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
rsmith added inline comments.
Comment at: lib/Sema/SemaType.cpp:7604-7608
+ // When instantiating a class template and reaching an atomic type, the check
+ // for type completeness should occur on the underlying type and not the
+ // atomic type itself (which is always incomplete).
+ if (inTemplateInstantiation() && T->isAtomicType())
+T = cast(T)->getValueType();
This function is duplicating the work of finding the type to complete, which
was already done by `isIncompleteType`. Rather than unwrapping `T` here...
Comment at: lib/Sema/SemaType.cpp:7636-7637
const TagType *Tag = T->getAs();
const ObjCInterfaceType *IFace = T->getAs();
... these uses of `T` should be `dyn_cast`ing `Def`, which is the declaration
that `isIncompleteType` said needed completing.
Comment at: lib/Sema/SemaType.cpp:7648-7649
//
// FIXME: Should we map through to the base array element type before
// checking for a tag type?
if (Tag || IFace) {
This would nicely address this FIXME, since `isIncompleteType` has already
walked through arrays.
Comment at: lib/Sema/SemaType.cpp:7679-7683
QualType MaybeTemplate = T;
while (const ConstantArrayType *Array
= Context.getAsConstantArrayType(MaybeTemplate))
MaybeTemplate = Array->getElementType();
if (const RecordType *Record = MaybeTemplate->getAs()) {
Likewise here, this should probably just be `if (const auto *RD =
dyn_cast(Def))`.
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.
Comment at: test/SemaCXX/atomic-type.cpp:5
template struct atomic {
- _Atomic(T) value;
+ _Atomic(T) value; // expected-error {{field has incomplete type
'_Atomic(user::inner)'}}
rsmith wrote:
> This is a bug. You need to teach `RequireCompleteType` to look through atomic
> types when looking for a class type to instantiate.
Thank you for the explanation -- I think I've addressed this in the latest
patch.
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman updated this revision to Diff 146524.
aaron.ballman added a comment.
Look through atomic types when checking type completeness during template
instantiation.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaType.cpp
test/CodeGen/c11atomics.c
test/Sema/atomic-type.c
test/SemaCXX/atomic-type.cpp
Index: test/SemaCXX/atomic-type.cpp
===
--- test/SemaCXX/atomic-type.cpp
+++ test/SemaCXX/atomic-type.cpp
@@ -94,3 +94,15 @@
bool PR21836(_Atomic(int) *x) {
return *x;
}
+
+struct T;
+
+void f(_Atomic T *t);
+
+struct T { int a, b; };
+
+void f(_Atomic T *t) {
+ T t2;
+ (void)__atomic_load(t, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(T) *' invalid)}}
+ (void)__c11_atomic_load(t, 5);
+}
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,22 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2; // expected-error {{tentative definition has type '_Atomic(struct ErrorS)' that is never completed}}
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+ _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
+
+typedef struct S S;
+void test_atomic_incomplete_struct_assignment(_Atomic S *s, _Atomic S *s2) {
+ *s = *s2; // expected-error {{incomplete type '_Atomic(S)' is not assignable}}
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
// we have to generate an atomic add, which returns the old value, and then a
// non-atomic add.
// CHECK: atomicrmw add i32* @i, i32 1 seq_cst
- // CHECK: add i32
+ // CHECK: add i32
++i;
// CHECK: atomicrmw add i64* @l, i64 1 seq_cst
// CHECK: add i64
@@ -475,6 +475,25 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+int test_atomic_void_ptr(_Atomic void *addr) {
+ // CHECK-LABEL: @test_atomic_void_ptr(
+ // CHECK: [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+ // CHECK: [[ATOMIC_RES:%.*]] = alloca i32, align 4
+ // CHECK: store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+ // CHECK: call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+ // CHECK: [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+ // CHECK: [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+ // CHECK: [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+ // CHECK: store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+ // CHECK: [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+ // CHECK: ret i32 [[RES]]
+ (void)__atomic_is_lock_free(1, addr);
+ return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
struct Empty {};
struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
@@ -488,3 +507,25 @@
// CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %{{.*}}, i32 5)
__c11_atomic_store(empty, value, 5);
}
+
+typedef struct T T;
+void test_struct_copy(_Atomic(T) *t1, _Atomic(T) *t2);
+struct T { int a, b; };
+void test_struct_copy(_Atomic(T) *t1, _Atomic(T) *t2) {
+ // CHECK-LABEL: @test_struct_copy(
+ // CHECK: [[T1:%.*]] = alloca %struct.T*, align 4
+ // CHECK: [[T2:%.*]] = alloca %struct.T*, align 4
+ // CHECK: [[TEMP:%.*]] = alloca %struct.T, align 8
+ // CHECK: store %struct.T* %t1, %struct.T** [[T1]], align 4
+ // CHECK:
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
rsmith added inline comments.
Comment at: test/SemaCXX/atomic-type.cpp:5
template struct atomic {
- _Atomic(T) value;
+ _Atomic(T) value; // expected-error {{field has incomplete type
'_Atomic(user::inner)'}}
This is a bug. You need to teach `RequireCompleteType` to look through atomic
types when looking for a class type to instantiate.
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman added a comment.
In https://reviews.llvm.org/D46112#1091981, @efriedma wrote:
> I think the request was that we check that a type is trivially copyable when
> we perform an atomic operation? I don't see the code for that anywhere.
Sorry about that -- I didn't notice that GNU was handled while C11 was not.
That's been updated now.
> Also needs some test coverage for atomic operations which aren't calls, like
> "typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2; };".
Thank you for pointing this out -- that uncovered an issue where we were not
properly diagnosing the types as being incomplete. I've added a new test case
and rolled the contents of Sema\atomic-type.cpp (which I added in an earlier
patch) into SemaCXX\atomic-type.cpp (which already existed and I missed it).
I believe the change I made to `Type::isIncompleteType()` is correct, but was
surprised by the new behavior in SemaCXX/atomic-type.cpp that resulted. It
appears that the `RecordDecl` for "struct inner" has `IsCompleteDefinition` set
to `false` while instantiating `struct atomic`, but I'm not familiar enough
with the template instantiation process to know whether this is reasonable or
not.
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman updated this revision to Diff 146474.
aaron.ballman added a comment.
Modifications based on review feedback.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/Type.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaType.cpp
test/CodeGen/c11atomics.c
test/Sema/atomic-type.c
test/SemaCXX/atomic-type.cpp
Index: test/SemaCXX/atomic-type.cpp
===
--- test/SemaCXX/atomic-type.cpp
+++ test/SemaCXX/atomic-type.cpp
@@ -2,17 +2,17 @@
// RUN: %clang_cc1 -verify -pedantic %s -std=c++11
template struct atomic {
- _Atomic(T) value;
+ _Atomic(T) value; // expected-error {{field has incomplete type '_Atomic(user::inner)'}}
void f() _Atomic; // expected-error {{expected ';' at end of declaration list}}
};
template struct user {
struct inner { char n[sizeof(T)]; };
- atomic i;
+ atomic i; // expected-note {{in instantiation}}
};
-user u;
+user u; // expected-note {{in instantiation}}
// Test overloading behavior of atomics.
struct A { };
@@ -94,3 +94,15 @@
bool PR21836(_Atomic(int) *x) {
return *x;
}
+
+struct T;
+
+void f(_Atomic T *t);
+
+struct T { int a, b; };
+
+void f(_Atomic T *t) {
+ T t2;
+ (void)__atomic_load(t, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(T) *' invalid)}}
+ (void)__c11_atomic_load(t, 5);
+}
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,22 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2; // expected-error {{tentative definition has type '_Atomic(struct ErrorS)' that is never completed}}
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+ _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
+
+typedef struct S S;
+void test_atomic_incomplete_struct_assignment(_Atomic S *s, _Atomic S *s2) {
+ *s = *s2; // expected-error {{incomplete type '_Atomic(S)' is not assignable}}
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
// we have to generate an atomic add, which returns the old value, and then a
// non-atomic add.
// CHECK: atomicrmw add i32* @i, i32 1 seq_cst
- // CHECK: add i32
+ // CHECK: add i32
++i;
// CHECK: atomicrmw add i64* @l, i64 1 seq_cst
// CHECK: add i64
@@ -475,6 +475,25 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+int test_atomic_void_ptr(_Atomic void *addr) {
+ // CHECK-LABEL: @test_atomic_void_ptr(
+ // CHECK: [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+ // CHECK: [[ATOMIC_RES:%.*]] = alloca i32, align 4
+ // CHECK: store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+ // CHECK: call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+ // CHECK: [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+ // CHECK: [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+ // CHECK: [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+ // CHECK: store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+ // CHECK: [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+ // CHECK: ret i32 [[RES]]
+ (void)__atomic_is_lock_free(1, addr);
+ return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
struct Empty {};
struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
@@ -488,3 +507,25 @@
// CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman updated this revision to Diff 146472.
aaron.ballman added a comment.
Addresses review comments.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
lib/Sema/SemaType.cpp
test/CodeGen/c11atomics.c
test/Sema/atomic-type.c
test/Sema/atomic-type.cpp
Index: test/Sema/atomic-type.cpp
===
--- test/Sema/atomic-type.cpp
+++ test/Sema/atomic-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S;
+
+void f(_Atomic S *s);
+
+struct S { int a, b; };
+
+void f(_Atomic S *s) {
+ S s2;
+ (void)__atomic_load(s, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(S) *' invalid)}}
+ (void)__c11_atomic_load(s, 5);
+}
+
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,17 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2;
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+ _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -61,7 +61,7 @@
// we have to generate an atomic add, which returns the old value, and then a
// non-atomic add.
// CHECK: atomicrmw add i32* @i, i32 1 seq_cst
- // CHECK: add i32
+ // CHECK: add i32
++i;
// CHECK: atomicrmw add i64* @l, i64 1 seq_cst
// CHECK: add i64
@@ -475,6 +475,25 @@
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+int test_atomic_void_ptr(_Atomic void *addr) {
+ // CHECK-LABEL: @test_atomic_void_ptr(
+ // CHECK: [[ADDR_ARG:%.*]] = alloca { i8, [1 x i8] }*, align 4
+ // CHECK: [[ATOMIC_RES:%.*]] = alloca i32, align 4
+ // CHECK: store { i8, [1 x i8] }* %addr, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE_VP:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR_LOCK_FREE_VP]] to i8*
+ // CHECK: call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+ // CHECK: [[ADDR:%.*]] = load { i8, [1 x i8] }*, { i8, [1 x i8] }** [[ADDR_ARG]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast { i8, [1 x i8] }* [[ADDR]] to i32*
+ // CHECK: [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+ // CHECK: [[CALL:%.*]] = call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+ // CHECK: store i32 [[CALL]], i32* [[ATOMIC_RES]], align 4
+ // CHECK: [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+ // CHECK: ret i32 [[RES]]
+ (void)__atomic_is_lock_free(1, addr);
+ return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
+
struct Empty {};
struct Empty test_empty_struct_load(_Atomic(struct Empty)* empty) {
@@ -488,3 +507,25 @@
// CHECK: call arm_aapcscc void @__atomic_store_1(i8* %{{.*}}, i8 zeroext %{{.*}}, i32 5)
__c11_atomic_store(empty, value, 5);
}
+
+typedef struct T T;
+void test_struct_copy(_Atomic(T) *t1, _Atomic(T) *t2);
+struct T { int a, b; };
+void test_struct_copy(_Atomic(T) *t1, _Atomic(T) *t2) {
+ // CHECK-LABEL: @test_struct_copy(
+ // CHECK: [[T1:%.*]] = alloca %struct.T*, align 4
+ // CHECK: [[T2:%.*]] = alloca %struct.T*, align 4
+ // CHECK: [[TEMP:%.*]] = alloca %struct.T, align 8
+ // CHECK: store %struct.T* %t1, %struct.T** [[T1]], align 4
+ // CHECK: store %struct.T* %t2, %struct.T** [[T2]], align 4
+ // CHECK: [[LOAD1:%.*]] = load %struct.T*, %struct.T** [[T1]], align 4
+ // CHECK: [[LOAD2:%.*]] = load %struct.T*, %struct.T** [[T2]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast %struct.T* [[LOAD2]] to i8*
+ // CHECK: [[CAST2:%.*]] = bitcast %struct.T* [[TEMP]] to i8*
+ // CHECK: call arm_aapcscc void @__atomic_load(i32 8,
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
efriedma added a comment.
I think the request was that we check that a type is trivially copyable when we
perform an atomic operation? I don't see the code for that anywhere.
Also needs some test coverage for atomic operations which aren't calls, like
"typedef struct S S; void f(_Atomic S *s, _Atomic S *s2) { *s = *s2; };".
https://reviews.llvm.org/D46112
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman added inline comments. Comment at: lib/Sema/SemaType.cpp:8022 +else if (LangOpts.CPlusPlus && isCompleteType(Loc, T) && + !T.isTriviallyCopyableType(Context)) // Some other non-trivially-copyable type (probably a C++ class) rsmith wrote: > efriedma wrote: > > If you're going to allow incomplete types in C++, you'll need some code to > > handle the case where the type is initially incomplete, but completed later. > Perhaps the trivially-copyable requirement could be deferred in all cases > until we reach an actual load or store. Constraints that are only enforced > when a type happens to be complete are generally a bad idea (we have a mess > like this for constraints on types being non-abstract, and we're still > working on unsticking ourselves from that mess in the C++ standard). > > In passing, I'd also note that we need the trivially-copyable requirement > even outside C++, for structs containing `__weak` / `__strong` fields. I'm not quite certain I've properly captured the request here, but I took a stab at it. One thing to note is that our behavior for type checking `__atomic_load()` appears to diverge from GCC with regards to trivial copyability, even before my patch. https://godbolt.org/g/g2fqLK https://reviews.llvm.org/D46112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman updated this revision to Diff 145409.
aaron.ballman added a comment.
Hopefully address the review comments.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
lib/Sema/SemaType.cpp
test/CodeGen/c11atomics.c
test/Sema/atomic-type.c
test/Sema/atomic-type.cpp
Index: test/Sema/atomic-type.cpp
===
--- test/Sema/atomic-type.cpp
+++ test/Sema/atomic-type.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only
+
+struct S;
+
+void f(_Atomic S *s);
+
+struct S { int a, b; };
+
+void f(_Atomic S *s) {
+ S s2;
+ (void)__atomic_load(s, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(S) *' invalid)}}
+ (void)__c11_atomic_load(s, 5);
+}
+
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,17 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2;
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
+
+void test_atomic_void_ptr(_Atomic void *addr) {
+ int i;
+ (void)__atomic_load(addr, , 5); // expected-error {{address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(void) *' invalid)}}
+ (void)__c11_atomic_load(addr, 5); // expected-error {{invalid use of incomplete type 'void'}}
+ _Static_assert(__atomic_is_lock_free(1, addr), "");
+}
Index: test/CodeGen/c11atomics.c
===
--- test/CodeGen/c11atomics.c
+++ test/CodeGen/c11atomics.c
@@ -474,3 +474,20 @@
// CHECK: ret i1 [[RES]]
return __c11_atomic_compare_exchange_strong(addr, desired, *new, 5, 5);
}
+
+int test_atomic_void_ptr(_Atomic void *addr) {
+ // CHECK-LABEL: @test_atomic_void_ptr(i8* %addr)
+ // CHECK: [[ADDR_ARG:%.*]] = alloca i8*, align 4
+ // CHECK: [[ATOMIC_RES:%.*]] = alloca i32, align 4
+ // CHECK: store i8* %addr, i8** [[ADDR_ARG]], align 4
+ // CHECK: [[ADDR_LOCK_FREE:%.*]] = load i8*, i8** [[ADDR_ARG]], align 4
+ // CHECK: call arm_aapcscc i32 @__atomic_is_lock_free(i32 1, i8* [[ADDR_LOCK_FREE]])
+ // CHECK: [[ADDR:%.*]] = load i8*, i8** [[ADDR_ARG]], align 4
+ // CHECK: [[CAST1:%.*]] = bitcast i8* [[ADDR]] to i32*
+ // CHECK: [[CAST2:%.*]] = bitcast i32* [[CAST1]] to i8*
+ // CHECK: call arm_aapcscc i32 @__atomic_load_4(i8* [[CAST2]], i32 5)
+ // CHECK: [[RES:%.*]] = load i32, i32* [[ATOMIC_RES]], align 4
+ // CHECK: ret i32 [[RES]]
+ (void)__atomic_is_lock_free(1, addr);
+ return (int)__c11_atomic_load((_Atomic int *)addr, 5);
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8019,25 +8019,21 @@
QualType Sema::BuildAtomicType(QualType T, SourceLocation Loc) {
if (!T->isDependentType()) {
-// FIXME: It isn't entirely clear whether incomplete atomic types
-// are allowed or not; for simplicity, ban them for the moment.
-if (RequireCompleteType(Loc, T, diag::err_atomic_specifier_bad_type, 0))
- return QualType();
-
int DisallowedKind = -1;
if (T->isArrayType())
- DisallowedKind = 1;
+ DisallowedKind = 0;
else if (T->isFunctionType())
- DisallowedKind = 2;
+ DisallowedKind = 1;
else if (T->isReferenceType())
- DisallowedKind = 3;
+ DisallowedKind = 2;
else if (T->isAtomicType())
- DisallowedKind = 4;
+ DisallowedKind = 3;
else if (T.hasQualifiers())
+ DisallowedKind = 4;
+else if (isCompleteType(Loc, T) && !T.isTriviallyCopyableType(Context))
+ // Some other non-trivially-copyable type (probably a C++ class, or a C
+ // struct with a __weak or __strong field).
DisallowedKind = 5;
-else if (!T.isTriviallyCopyableType(Context))
- // Some other non-trivially-copyable type (probably a C++ class)
- DisallowedKind = 6;
if (DisallowedKind != -1) {
Diag(Loc, diag::err_atomic_specifier_bad_type) << DisallowedKind << T;
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
rsmith added inline comments. Comment at: lib/Sema/SemaType.cpp:8022 +else if (LangOpts.CPlusPlus && isCompleteType(Loc, T) && + !T.isTriviallyCopyableType(Context)) // Some other non-trivially-copyable type (probably a C++ class) efriedma wrote: > If you're going to allow incomplete types in C++, you'll need some code to > handle the case where the type is initially incomplete, but completed later. Perhaps the trivially-copyable requirement could be deferred in all cases until we reach an actual load or store. Constraints that are only enforced when a type happens to be complete are generally a bad idea (we have a mess like this for constraints on types being non-abstract, and we're still working on unsticking ourselves from that mess in the C++ standard). In passing, I'd also note that we need the trivially-copyable requirement even outside C++, for structs containing `__weak` / `__strong` fields. https://reviews.llvm.org/D46112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
efriedma added a comment. This generally makes sense. Need some tests to make sure we emit an appropriate error if you try to actually use atomic operators (load/store) or intrinsics (__atomic_is_lock_free etc.) with an incomplete type. And a test that code generation emits something appropriate. Comment at: lib/Sema/SemaType.cpp:8022 +else if (LangOpts.CPlusPlus && isCompleteType(Loc, T) && + !T.isTriviallyCopyableType(Context)) // Some other non-trivially-copyable type (probably a C++ class) If you're going to allow incomplete types in C++, you'll need some code to handle the case where the type is initially incomplete, but completed later. https://reviews.llvm.org/D46112 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46112: Allow _Atomic to be specified on incomplete types
aaron.ballman created this revision.
aaron.ballman added a reviewer: rsmith.
The C standard does not prohibit the _Atomic specifier on incomplete types,
which turns out to be sometimes useful.
This addresses PR37237.
Note that C++ still requires checking if the type is complete in order to
support member pointers under the Microsoft ABI (because that check completes
some information on the type), which is a use case we have in
test\SemaCXX\atomic-type.cpp.
https://reviews.llvm.org/D46112
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaType.cpp
test/Sema/atomic-type.c
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,10 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to
function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied
to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2;
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to
array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to
qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to
atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of
'struct Incomplete' will not be visible outside of this function}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8007,25 +8007,21 @@
QualType Sema::BuildAtomicType(QualType T, SourceLocation Loc) {
if (!T->isDependentType()) {
-// FIXME: It isn't entirely clear whether incomplete atomic types
-// are allowed or not; for simplicity, ban them for the moment.
-if (RequireCompleteType(Loc, T, diag::err_atomic_specifier_bad_type, 0))
- return QualType();
-
int DisallowedKind = -1;
if (T->isArrayType())
- DisallowedKind = 1;
+ DisallowedKind = 0;
else if (T->isFunctionType())
- DisallowedKind = 2;
+ DisallowedKind = 1;
else if (T->isReferenceType())
- DisallowedKind = 3;
+ DisallowedKind = 2;
else if (T->isAtomicType())
- DisallowedKind = 4;
+ DisallowedKind = 3;
else if (T.hasQualifiers())
- DisallowedKind = 5;
-else if (!T.isTriviallyCopyableType(Context))
+ DisallowedKind = 4;
+else if (LangOpts.CPlusPlus && isCompleteType(Loc, T) &&
+ !T.isTriviallyCopyableType(Context))
// Some other non-trivially-copyable type (probably a C++ class)
- DisallowedKind = 6;
+ DisallowedKind = 5;
if (DisallowedKind != -1) {
Diag(Loc, diag::err_atomic_specifier_bad_type) << DisallowedKind << T;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5504,7 +5504,7 @@
"incomplete result type %0 in function definition">;
def err_atomic_specifier_bad_type : Error<
"_Atomic cannot be applied to "
- "%select{incomplete |array |function |reference |atomic |qualified |}0type "
+ "%select{array |function |reference |atomic |qualified |}0type "
"%1 %select{||which is not trivially copyable}0">;
// Expressions.
Index: test/Sema/atomic-type.c
===
--- test/Sema/atomic-type.c
+++ test/Sema/atomic-type.c
@@ -16,7 +16,10 @@
extern _Atomic(int (*)(int(*)[10], int(*)[10])) mergetest;
_Atomic(int()) error1; // expected-error {{_Atomic cannot be applied to function type}}
-_Atomic(struct ErrorS) error2; // expected-error {{_Atomic cannot be applied to incomplete type}} expected-note {{forward declaration}}
+_Atomic(struct ErrorS) error2;
_Atomic(int[10]) error3; // expected-error {{_Atomic cannot be applied to array type}}
_Atomic(const int) error4; // expected-error {{_Atomic cannot be applied to qualified type}}
_Atomic(_Atomic(int)) error5; // expected-error {{_Atomic cannot be applied to atomic type}}
+
+void g(_Atomic void *ptr);
+void h(_Atomic struct Incomplete *ptr); // expected-warning {{declaration of 'struct Incomplete' will not be visible outside of this function}}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -8007,25 +8007,21 @@
QualType Sema::BuildAtomicType(QualType T, SourceLocation Loc) {
if (!T->isDependentType()) {
-// FIXME: It isn't entirely clear whether incomplete atomic types
-// are allowed or not; for simplicity, ban them for the moment.
-if (RequireCompleteType(Loc, T, diag::err_atomic_specifier_bad_type, 0))
- return
