[PATCH] PR23923: "redefine_extname" pragma handled incorrectly in case of a local var with the same name

[Aaron Ballman]

On Wed, Jun 24, 2015 at 10:54 AM, Andrey Bokhanko
<andreybokhanko at gmail.com> wrote:
> Patch updated in response to Aaron Ballman's comment (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131626.html).
>
> Aaron, everything fixed according to your comments (including adding a new test).
>
>
> http://reviews.llvm.org/D10646
>
> 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
> @@ -8,11 +8,21 @@
>    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
>  // pragma, subsequent function name redefined properly. PR5172, Comment 11.
>  // 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 int foo() { return 1; }
> +// CHECK: define i32 @bar()

This is not testing what I was hoping to test. I wanted to know if a
declaration of foo() in an extern "C" language linkage block is
handled correctly. Instead, try:

extern "C" {
  int foo() { return 1; }
}

And then (I presume), CHECK: define i32 @bar()

Other changes look good though.

Thank you!

~Aaron

> +
> Index: test/CodeGen/redefine_extname.c
> ===================================================================
> --- test/CodeGen/redefine_extname.c
> +++ test/CodeGen/redefine_extname.c
> @@ -13,3 +13,14 @@
>  // 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()
> +
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -5522,6 +5522,19 @@
>    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();
> +  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,
> @@ -5946,7 +5959,8 @@
>
>      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()) {
> @@ -7468,7 +7482,8 @@
>      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()) {
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/