[PATCH] D46112: Allow _Atomic to be specified on incomplete types

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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<AtomicType>(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<TagType>();
   const ObjCInterfaceType *IFace = T->getAs<ObjCInterfaceType>();
 
----------------
... 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<RecordType>()) {
----------------
Likewise here, this should probably just be `if (const auto *RD = dyn_cast<RecordDecl>(Def))`.


https://reviews.llvm.org/D46112