[cfe-commits] r64413 - in /cfe/trunk: include/clang/AST/DeclBase.h lib/CodeGen/CGDecl.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CMakeLists.txt lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenModule.cpp lib/CodeGen/CodeGenModule.h lib/CodeGen/Mangle.cpp lib/CodeGen/Mangle.h test/CodeGen/overloadable.c test/CodeGenCXX/mangle.cpp

[Douglas Gregor]

On Feb 17, 2009, at 4:18 PM, Chris Lattner wrote:

> On Feb 12, 2009, at 4:10 PM, Douglas Gregor wrote:
>> URL: http://llvm.org/viewvc/llvm-project?rev=64413&view=rev
>> Log:
>> Add basic support for C++ name mangling according to the Itanium C++
>> ABI to the CodeGen library. Since C++ code-generation is so
>> incomplete, we can't exercise much of this mangling code. However, a
>> few smoke tests show that it's doing the same thing as GCC. When C++
>> codegen matures, we'll extend the ABI tester to verify name-mangling
>> as well, and complete the implementation here.
>
> Very nice Doug, thank you for working on this!
>
>> +/// \brief Retrieves the mangled name for the given declaration.
>> +///
>> +/// If the given declaration requires a mangled name, returns an
>> +/// IdentifierInfo* containing the mangled name. Otherwise, returns
>> +/// the name of the declaration as an identifier.
>> +///
>> +/// FIXME: Returning an IdentifierInfo* here is a total hack. We
>
> Yes, yes it is.  Please use a llvm/ADT/StringSet.h in CodeGenModule  
> to hold these.

Okay, done.

>> +IdentifierInfo *CodeGenModule::getMangledName(const NamedDecl *ND)  
>> const {
>> +  std::string Name;
>
> Also, please use a SmallString<256> or something like that to avoid  
> heap traffic.

Ah, cool. Done.

>> @@ -0,0 +1,516 @@
>> +//===--------------------- Mangle.cpp - Mangle C++ Names  
>> ------------------===//
>
> Any reason to center this?

It's fun?

(Fixed)

>> +bool CXXNameMangler::mangle(const NamedDecl *D) {
>> +  // <mangled-name> ::= _Z <encoding>
>> +  //            ::= <data name>
>> +  //            ::= <special-name>
>> +
>> +  // FIXME: Actually use a visitor to decode these?
>> +  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
>> +    bool RequiresMangling = false;
>> +    // Clang's "overloadable" attribute extension to C/C++
>> +    if (FD->getAttr<OverloadableAttr>())
>> +      RequiresMangling = true;
>> +    else if (Context.getLangOptions().CPlusPlus) {
>> +      RequiresMangling = true;
>> +      if (isa<LinkageSpecDecl>(FD->getDeclContext()) &&
>> +          cast<LinkageSpecDecl>(FD->getDeclContext())->getLanguage()
>> +            == LinkageSpecDecl::lang_c) {
>
> IS:
>
> extern "C" {
>  namespace foo {
>    int bar();
>  }
> }
>
> legal?  If so, you'd have to check all parent contexts for an extern  
> c block.

Yep, you're right.

>  Also, you should check SourceManager::getFileCharacteristic on the  
> decl's location to see if it is in an "implicit extern C system  
> header".

Didn't know about that. Okay, thanks.

Linkage is still a bit messy. Rather than trying to discern linkage  
here in name mangling or elsewhere in CodeGen, we should be annotating  
Decls (and function types!) with their linkage. This has been  
bothering me for a while, but hasn't yet bubbled to the top. Maybe  
soon :)

>> +static bool isStdNamespace(const DeclContext *DC) {
>> +  if (!DC->isNamespace() || !DC->getParent()->isTranslationUnit())
>> +    return false;
>> +
>> +  const NamespaceDecl *NS = cast<NamespaceDecl>(DC);
>> +  const IdentifierInfo *Name = NS->getIdentifier();
>> +  if (Name->getLength() != 3)
>> +    return false;
>> +
>> +  const char *Str = Name->getName();
>> +  if (Str[0] != 's' || Str[1] != 't' || Str[2] != 'd')
>> +    return false;
>
> return NS->getIdentifier()->isStr("std");
>
> should be enough.

Okay.

>> +  case NUM_OVERLOADED_OPERATORS:
>> +    assert(false && "Not an ovelroaded operator");
>
> typo

Thanks.

>> @@ -0,0 +1,32 @@
>> +//===---------------------- Mangle.h - Mangle C++ Names  
>> -------------------===//
>
> Likewise, centering.

Fixed.

	- Doug