r240653 - Fix #pragma redefine_extname when there is a local variable of the same name. The local should not be renamed, only the externally-available declaration should be.

[Andrey Bokhanko]

Richard is right (as always!) -- GCC doesn't apply this pragma to C++
code. I believe original pragma authors had only C language in mind.

I'm off my work computer through the rest of the weekend, but will get
back on Monday and cook up a fix (most likely just checking isExternC
would be enough... but obviously, I have to test).

Yours,
Andrey Bokhanko
==============
Software Engineer
Intel Compiler Team
Intel

On Fri, Jun 26, 2015 at 12:52 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Thu, Jun 25, 2015 at 5:43 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> On Thu, Jun 25, 2015 at 8:37 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>>>
>>> Author: aaronballman
>>> Date: Thu Jun 25 10:37:16 2015
>>> New Revision: 240653
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=240653&view=rev
>>> Log:
>>> Fix #pragma redefine_extname when there is a local variable of the same
>>> name. The local should not be renamed, only the externally-available
>>> declaration should be.
>>>
>>> Patch by Andrey Bokhanko!
>>>
>>> Modified:
>>>     cfe/trunk/lib/Sema/SemaDecl.cpp
>>>     cfe/trunk/test/CodeGen/redefine_extname.c
>>>     cfe/trunk/test/CodeGenCXX/redefine_extname.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=240653&r1=240652&r2=240653&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Jun 25 10:37:16 2015
>>> @@ -5525,6 +5525,19 @@ bool Sema::adjustContextForLocalExternDe
>>>    return true;
>>>  }
>>>
>>> +/// \brief Returns true if given declaration is TU-scoped and externally
>>> +/// visible.
>>> +static bool isDeclTUScopedExternallyVisible(const Decl *D) {
>>> +  if (auto *FD = dyn_cast<FunctionDecl>(D))
>>> +    return (FD->getDeclContext()->isTranslationUnit() || FD->isExternC())
>>> &&
>>> +           FD->hasExternalFormalLinkage();
>>
>>
>> Hmm, should non-extern-C functions get this behavior at all in C++? My
>> understanding is that redefine_extname is supposed to have the semantics of
>> "search and replace on the generated assembly", so it should only apply to
>> names that don't get mangled.
>
> Possibly. Andrey, what do you think?
>
> ~Aaron
>
>>
>>>
>>> +  else if (auto *VD = dyn_cast<VarDecl>(D))
>>> +    return (VD->getDeclContext()->isTranslationUnit() || VD->isExternC())
>>> &&
>>> +           VD->hasExternalFormalLinkage();
>>> +
>>> +  llvm_unreachable("Unknown type of decl!");
>>> +}
>>> +
>>>  NamedDecl *
>>>  Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
>>>                                TypeSourceInfo *TInfo, LookupResult
>>> &Previous,
>>> @@ -5949,7 +5962,8 @@ Sema::ActOnVariableDeclarator(Scope *S,
>>>
>>>      NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0),
>>>                                                  Context, Label, 0));
>>> -  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
>>> +  } else if (!ExtnameUndeclaredIdentifiers.empty() &&
>>> +             isDeclTUScopedExternallyVisible(NewVD)) {
>>>      llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
>>>        ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
>>>      if (I != ExtnameUndeclaredIdentifiers.end()) {
>>> @@ -7471,7 +7485,8 @@ Sema::ActOnFunctionDeclarator(Scope *S,
>>>      StringLiteral *SE = cast<StringLiteral>(E);
>>>      NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0),
>>> Context,
>>>                                                  SE->getString(), 0));
>>> -  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
>>> +  } else if (!ExtnameUndeclaredIdentifiers.empty() &&
>>> +             isDeclTUScopedExternallyVisible(NewFD)) {
>>>      llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
>>>        ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
>>>      if (I != ExtnameUndeclaredIdentifiers.end()) {
>>>
>>> Modified: cfe/trunk/test/CodeGen/redefine_extname.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/redefine_extname.c?rev=240653&r1=240652&r2=240653&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGen/redefine_extname.c (original)
>>> +++ cfe/trunk/test/CodeGen/redefine_extname.c Thu Jun 25 10:37:16 2015
>>> @@ -13,3 +13,14 @@ int fish() { return fake() + __PRAGMA_RE
>>>  // CHECK:   call i32 @real()
>>>  // Check that this also works with variables names
>>>  // CHECK:   load i32, i32* @alias
>>> +
>>> +// This is a case when redefenition is deferred *and* we have a local of
>>> the
>>> +// same name. PR23923.
>>> +#pragma redefine_extname foo bar
>>> +int f() {
>>> +  int foo = 0;
>>> +  return foo;
>>> +}
>>> +extern int foo() { return 1; }
>>> +// CHECK: define i32 @bar()
>>> +
>>>
>>> Modified: cfe/trunk/test/CodeGenCXX/redefine_extname.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/redefine_extname.cpp?rev=240653&r1=240652&r2=240653&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGenCXX/redefine_extname.cpp (original)
>>> +++ cfe/trunk/test/CodeGenCXX/redefine_extname.cpp Thu Jun 25 10:37:16
>>> 2015
>>> @@ -8,7 +8,7 @@ extern "C" {
>>>    int statvfs64(struct statvfs64 *);
>>>  }
>>>
>>> -void foo() {
>>> +void some_func() {
>>>    struct statvfs64 st;
>>>    statvfs64(&st);
>>>  // Check that even if there is a structure with redefined name before the
>>> @@ -16,3 +16,15 @@ void foo() {
>>>  // CHECK:  call i32 @statvfs(%struct.statvfs64* %st)
>>>  }
>>>
>>> +// This is a case when redefenition is deferred *and* we have a local of
>>> the
>>> +// same name. PR23923.
>>> +#pragma redefine_extname foo bar
>>> +int f() {
>>> +  int foo = 0;
>>> +  return foo;
>>> +}
>>> +extern "C" {
>>> +  int foo() { return 1; }
>>> +// CHECK: define i32 @bar()
>>> +}
>>> +
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>