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, &t2, 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, &i, 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) {
@@ -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, i8* [[CAST1]], i8* [[CAST2]], i32 5)
+  // CHECK:  [[CAST3:%.*]] = bitcast %struct.T* [[LOAD1]] to i8*
+  // CHECK:  [[CAST4:%.*]] = bitcast %struct.T* [[TEMP]] to i8*
+  // CHECK:  call arm_aapcscc void @__atomic_store(i32 8, i8* [[CAST3]], i8* [[CAST4]], i32 5)
+  // CHECK:  ret void
+  *t1 = *t2;
+}
Index: test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
===================================================================
--- test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
+++ test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
@@ -108,7 +108,7 @@
   extern int incomplete[];
   for (auto a : incomplete) // expected-error {{cannot use incomplete type 'int []' as a range}}
     ;
-  extern struct Incomplete also_incomplete[2]; // expected-note {{forward declaration}}
+  extern struct Incomplete also_incomplete[2]; // expected-note 2{{forward declaration}}
   for (auto &a : also_incomplete) // expected-error {{cannot use incomplete type 'struct Incomplete [2]' as a range}}
     ;
 
@@ -152,7 +152,7 @@
   };
   for (auto u : NoBegin()) { // expected-error {{range type 'NoBegin' has 'end' member but no 'begin' member}}
   }
-  for (auto u : NoEnd()) { // expected-error {{range type 'NoEnd' has 'begin' member but no 'end' member}} 
+  for (auto u : NoEnd()) { // expected-error {{range type 'NoEnd' has 'begin' member but no 'end' member}}
   }
 
   struct NoIncr {
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7627,38 +7627,27 @@
     return false;
   }
 
-  const TagType *Tag = T->getAs<TagType>();
-  const ObjCInterfaceType *IFace = T->getAs<ObjCInterfaceType>();
-
   // If there's an unimported definition of this type in a module (for
   // instance, because we forward declared it, then imported the definition),
   // import that definition now.
   //
   // FIXME: What about other cases where an import extends a redeclaration
   // chain for a declaration that can be accessed through a mechanism other
   // than name lookup (eg, referenced in a template, or a variable whose type
   // could be completed by the module)?
-  //
-  // FIXME: Should we map through to the base array element type before
-  // checking for a tag type?
-  if (Tag || IFace) {
-    NamedDecl *D =
-        Tag ? static_cast<NamedDecl *>(Tag->getDecl()) : IFace->getDecl();
-
+  if (Def) {
     // Avoid diagnosing invalid decls as incomplete.
-    if (D->isInvalidDecl())
+    if (Def->isInvalidDecl())
       return true;
 
     // Give the external AST source a chance to complete the type.
     if (auto *Source = Context.getExternalSource()) {
-      if (Tag) {
-        TagDecl *TagD = Tag->getDecl();
+      if (auto *TagD = dyn_cast<TagDecl>(Def)) {
         if (TagD->hasExternalLexicalStorage())
           Source->CompleteType(TagD);
-      } else {
-        ObjCInterfaceDecl *IFaceD = IFace->getDecl();
+      } else if (auto *IFaceD = dyn_cast<ObjCInterfaceDecl>(Def)) {
         if (IFaceD->hasExternalLexicalStorage())
-          Source->CompleteType(IFace->getDecl());
+          Source->CompleteType(IFaceD);
       }
       // If the external source completed the type, go through the motions
       // again to ensure we're allowed to use the completed type.
@@ -7670,23 +7659,18 @@
   // If we have a class template specialization or a class member of a
   // class template specialization, or an array with known size of such,
   // try to instantiate it.
-  QualType MaybeTemplate = T;
-  while (const ConstantArrayType *Array
-           = Context.getAsConstantArrayType(MaybeTemplate))
-    MaybeTemplate = Array->getElementType();
-  if (const RecordType *Record = MaybeTemplate->getAs<RecordType>()) {
+  if (auto *RD = dyn_cast_or_null<RecordDecl>(Def)) {
     bool Instantiated = false;
     bool Diagnosed = false;
-    if (ClassTemplateSpecializationDecl *ClassTemplateSpec
-          = dyn_cast<ClassTemplateSpecializationDecl>(Record->getDecl())) {
+    if (auto *ClassTemplateSpec =
+            dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
       if (ClassTemplateSpec->getSpecializationKind() == TSK_Undeclared) {
         Diagnosed = InstantiateClassTemplateSpecialization(
             Loc, ClassTemplateSpec, TSK_ImplicitInstantiation,
             /*Complain=*/Diagnoser);
         Instantiated = true;
       }
-    } else if (CXXRecordDecl *Rec
-                 = dyn_cast<CXXRecordDecl>(Record->getDecl())) {
+    } else if (auto *Rec = dyn_cast<CXXRecordDecl>(RD)) {
       CXXRecordDecl *Pattern = Rec->getInstantiatedFromMemberClass();
       if (!Rec->isBeingDefined() && Pattern) {
         MemberSpecializationInfo *MSI = Rec->getMemberSpecializationInfo();
@@ -7724,17 +7708,19 @@
 
   Diagnoser->diagnose(*this, Loc, T);
 
-  // If the type was a forward declaration of a class/struct/union
-  // type, produce a note.
-  if (Tag && !Tag->getDecl()->isInvalidDecl())
-    Diag(Tag->getDecl()->getLocation(),
-         Tag->isBeingDefined() ? diag::note_type_being_defined
-                               : diag::note_forward_declaration)
-      << QualType(Tag, 0);
-
-  // If the Objective-C class was a forward declaration, produce a note.
-  if (IFace && !IFace->getDecl()->isInvalidDecl())
-    Diag(IFace->getDecl()->getLocation(), diag::note_forward_class);
+  if (Def && !Def->isInvalidDecl()) {
+    // If the type was a forward declaration of a class/struct/union
+    // type, produce a note.
+    if (const auto *TD = dyn_cast<TagDecl>(Def))
+      Diag(TD->getLocation(), TD->isBeingDefined()
+                                  ? diag::note_type_being_defined
+                                  : diag::note_forward_declaration)
+          << QualType(TD->getTypeForDecl(), 0);
+
+    // If the Objective-C class was a forward declaration, produce a note.
+    if (isa<ObjCInterfaceDecl>(Def))
+      Diag(Def->getLocation(), diag::note_forward_class);
+  }
 
   // If we have external information that we can use to suggest a fix,
   // produce a note.
@@ -8019,25 +8005,17 @@
 
 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))
-      // Some other non-trivially-copyable type (probably a C++ class)
-      DisallowedKind = 6;
+      DisallowedKind = 4;
 
     if (DisallowedKind != -1) {
       Diag(Loc, diag::err_atomic_specifier_bad_type) << DisallowedKind << T;
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3144,7 +3144,7 @@
     return ExprError();
   }
 
-  // For a __c11 builtin, this should be a pointer to an _Atomic type.
+  // For a __c11 builtin, this should be a pointer to a complete _Atomic type.
   QualType AtomTy = pointerType->getPointeeType(); // 'A'
   QualType ValType = AtomTy; // 'C'
   if (IsC11) {
@@ -3161,6 +3161,10 @@
       return ExprError();
     }
     ValType = AtomTy->getAs<AtomicType>()->getValueType();
+
+    if (RequireCompleteType(DRE->getLocStart(), ValType,
+                            diag::err_invalid_incomplete_type_use, ValType))
+      return ExprError();
   } else if (Form != Load && Form != LoadCopy) {
     if (ValType.isConstQualified()) {
       Diag(DRE->getLocStart(), diag::err_atomic_op_needs_non_const_pointer)
@@ -3195,12 +3199,11 @@
     return ExprError();
   }
 
-  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
+    // atomics specification, but we enforce it for sanity.
     Diag(DRE->getLocStart(), diag::err_atomic_op_needs_trivial_copy)
-      << Ptr->getType() << Ptr->getSourceRange();
+        << Ptr->getType() << Ptr->getSourceRange();
     return ExprError();
   }
 
Index: lib/AST/Type.cpp
===================================================================
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2000,6 +2000,14 @@
     // Void is the only incomplete builtin type.  Per C99 6.2.5p19, it can never
     // be completed.
     return isVoidType();
+  case Atomic:
+    // Atomic types do not have to have the same size and alignment
+    // requirements as their underlying type, however, for determining
+    // completeness, if the underlying type is incomplete, the atomic-qualified
+    // type cannot be complete.
+    return cast<AtomicType>(CanonicalType)
+        ->getValueType()
+        ->isIncompleteType(Def);
   case Enum: {
     EnumDecl *EnumD = cast<EnumType>(CanonicalType)->getDecl();
     if (Def)
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5513,8 +5513,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 "
-  "%1 %select{||||||which is not trivially copyable}0">;
+  "%select{array|function|reference|atomic|qualified}0 type %1">;
 
 // Expressions.
 def ext_sizeof_alignof_function_type : Extension<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to