[cfe-commits] [PATCH] Add _Atomic specifier for atomic types

[John McCall]

On Oct 4, 2011, at 11:47 AM, Jeffrey Yasskin wrote:
> On Mon, Oct 3, 2011 at 7:53 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> Per subject; after some discussions with John, I decided that we
>> really want to be prepared for C1x atomics, and being able to use
>> _Atomic in the implementation of <atomic> will make the implementation
>> of <atomic> more flexible.  This patch follows the syntax from the C1x
>> draft which will hopefully stick: _Atomic(int*) specifies an atomic
>> pointer to an integer, etc.
> 
> The C1x draft makes _Atomic(type) a type specifier and plain _Atomic a
> type qualifier. It looks like your patch only implements the type
> specifier. Was that intentional?

Well, the type specifier is clearly non-optional, but there are outstanding
draft comments recommending the removal of the type qualifier.

>> +  if (LHSType->isAtomicType())
>> +    return Incompatible;
>> +
>>   // Common case: no conversion required.
>>   if (LHSType == RHSType) {
>>     Kind = CK_NoOp;
>> Index: lib/AST/TypePrinter.cpp
>> ===================================================================
>> --- lib/AST/TypePrinter.cpp	(revision 141016)
>> +++ lib/AST/TypePrinter.cpp	(working copy)
>> @@ -581,6 +582,16 @@
>>   }
>> }
>> 
>> +void TypePrinter::printAtomic(const AtomicType *T, std::string &S) {
>> +  if (!S.empty())
>> +    S = ' ' + S;
>> +  std::string Str;
>> +  IncludeStrongLifetimeRAII Strong(Policy);
>> +  print(T->getValueType(), Str);
>> +
>> +  S = "_Atomic(" + Str + ")" + S;
> 
> Wow, this is inefficient.  I wouldn't comment, but printAtomic takes
> an out parameter for the result, and the only reason to do that is to
> avoid copies, but this code copies out the wazoo.

All the type-printing stuff does things like this.  I agree that we should
probably use a different string data structure, but for now, it's unavoidable.

> Does "The type name in an atomic type specifier shall not refer to an
> array type" from C1x 6.7.2.4 imply this should go in the "type should
> never be variably-modified" section?

Variably-modified is more than just VLAs;  it's "mentions a VLA
somewhere".  In principle, you can have an atomic pointer to VLA type.

>> +      break;
>>     }
>>   } while (type->isVariablyModifiedType());
>> }
>> Index: lib/CodeGen/CGRTTI.cpp
>> ===================================================================
>> --- lib/CodeGen/CGRTTI.cpp	(revision 141016)
>> +++ lib/CodeGen/CGRTTI.cpp	(working copy)
>> @@ -474,6 +474,10 @@
>>     // abi::__pointer_to_member_type_info.
>>     VTableName = "_ZTVN10__cxxabiv129__pointer_to_member_type_infoE";
>>     break;
>> +
>> +  case Type::Atomic:
>> +    // Pretend this is a class; not sure what else we can do here.
> 
> FIXME: ask the C++ ABI folks?  Complex and vector get to be
> fundamental types, so maybe atomic should be too?

Yeah, I think calling it a fundamental type is appropriate.

John.