[PATCH] Disabling of "redefine_extname" pragma for C++ code

Andrey Bokhanko andreybokhanko at gmail.com
Tue Jul 7 07:42:01 PDT 2015


Aaron, thank you for the review!

I added a warning (for non applying pragma redefine_extname) and
removed these nasty braces. :-)

Please re-review: http://reviews.llvm.org/D10805.

Yours,
Andrey


On Mon, Jul 6, 2015 at 3:23 PM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
> On Mon, Jun 29, 2015 at 11:13 AM, Andrey Bokhanko
> <andreybokhanko at gmail.com> wrote:
>> Hi aaron.ballman, rsmith,
>>
>> In response to Richard Smith's comment (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131782.html), this patch disables "redefine_extname" pragma for C++ code. Also, I added a test that this pragma doesn't apply to static declarations.
>>
>> http://reviews.llvm.org/D10805
>>
>> Files:
>>   lib/Sema/SemaDecl.cpp
>>   test/CodeGen/redefine_extname.c
>>   test/CodeGenCXX/redefine_extname.cpp
>>
>> Index: test/CodeGenCXX/redefine_extname.cpp
>> ===================================================================
>> --- test/CodeGenCXX/redefine_extname.cpp
>> +++ test/CodeGenCXX/redefine_extname.cpp
>> @@ -28,3 +28,9 @@
>>  // CHECK: define i32 @bar()
>>  }
>>
>> +// Check that pragma redefine_extname applies to C code only, and shouldn't be
>> +// applied to C++.
>> +#pragma redefine_extname foo_cpp bar_cpp
>> +extern int foo_cpp() { return 1; }
>> +// CHECK-NOT: define i32 @bar_cpp()
>> +
>> Index: test/CodeGen/redefine_extname.c
>> ===================================================================
>> --- test/CodeGen/redefine_extname.c
>> +++ test/CodeGen/redefine_extname.c
>> @@ -24,3 +24,9 @@
>>  extern int foo() { return 1; }
>>  // CHECK: define i32 @bar()
>>
>> +// Check that pragma redefine_extname applies to external declarations only.
>> +#pragma redefine_extname foo_static bar_static
>> +static int foo_static() { return 1; }
>> +int baz() { return foo_static(); }
>> +// CHECK-NOT: call i32 @bar_static()
>
> Since this is becoming more and more specific behavior, should this
> warn when there are *no* declarations renamed, but there are
> declarations with the same name? I can see wasting a lot of time
> trying to debug the above test case.
>
>> +
>> Index: lib/Sema/SemaDecl.cpp
>> ===================================================================
>> --- lib/Sema/SemaDecl.cpp
>> +++ lib/Sema/SemaDecl.cpp
>> @@ -5525,15 +5525,16 @@
>>    return true;
>>  }
>>
>> -/// \brief Returns true if given declaration is TU-scoped and externally
>> -/// visible.
>> -static bool isDeclTUScopedExternallyVisible(const Decl *D) {
>> +/// \brief Returns true if given declaration has external C language linkage.
>> +static bool isDeclExternC(const Decl *D) {
>>    if (auto *FD = dyn_cast<FunctionDecl>(D))
>> -    return (FD->getDeclContext()->isTranslationUnit() || FD->isExternC()) &&
>> -           FD->hasExternalFormalLinkage();
>> +  {
>> +    return FD->isExternC();
>> +  }
>
> Braces go with the if, not on a new line.
>
>>    else if (auto *VD = dyn_cast<VarDecl>(D))
>> -    return (VD->getDeclContext()->isTranslationUnit() || VD->isExternC()) &&
>> -           VD->hasExternalFormalLinkage();
>> +  {
>> +    return VD->isExternC();
>> +  }
>
> Same here.
>
>>
>>    llvm_unreachable("Unknown type of decl!");
>>  }
>> @@ -5963,7 +5964,7 @@
>>      NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0),
>>                                                  Context, Label, 0));
>>    } else if (!ExtnameUndeclaredIdentifiers.empty() &&
>> -             isDeclTUScopedExternallyVisible(NewVD)) {
>> +             isDeclExternC(NewVD)) {
>>      llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
>>        ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
>>      if (I != ExtnameUndeclaredIdentifiers.end()) {
>> @@ -7491,7 +7492,7 @@
>>      NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0), Context,
>>                                                  SE->getString(), 0));
>>    } else if (!ExtnameUndeclaredIdentifiers.empty() &&
>> -             isDeclTUScopedExternallyVisible(NewFD)) {
>> +             isDeclExternC(NewFD)) {
>>      llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
>>        ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
>>      if (I != ExtnameUndeclaredIdentifiers.end()) {
>> @@ -14284,7 +14285,7 @@
>>    // already exists, add a label attribute to it.
>>    if (PrevDecl &&
>>        (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl)) &&
>> -      PrevDecl->hasExternalFormalLinkage())
>> +      isDeclExternC(PrevDecl))
>>      PrevDecl->addAttr(Attr);
>>    // Otherwise, add a label atttibute to ExtnameUndeclaredIdentifiers.
>>    else
>
> ~Aaron
>
>>
>> EMAIL PREFERENCES
>>   http://reviews.llvm.org/settings/panel/emailpreferences/




More information about the cfe-commits mailing list