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

[Aaron Ballman]

On Tue, Jun 23, 2015 at 7:03 AM, Andrey Bokhanko
<andreybokhanko at gmail.com> wrote:
> Hi rsmith, aaron.ballman,
>
> As Richard Smith noted in his review (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150608/130447.html), pragma "redefine_extname" doesn't work correctly when there is a local of the same name. The patch fixes this.
>
> http://reviews.llvm.org/D10646
>
> Files:
>   lib/Sema/SemaDecl.cpp
>   test/CodeGen/redefine_extname.c
>
> 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
> @@ -5946,7 +5946,11 @@
>
>      NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0),
>                                                  Context, Label, 0));
> -  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
> +  // Deferred handling of "redefine_extname" pragma.
> +  } else if (!ExtnameUndeclaredIdentifiers.empty() &&
> +             (NewVD->getDeclContext()->isTranslationUnit() ||
> +              NewVD->isExternC()) &&
> +             NewVD->hasExternalFormalLinkage()) {
>      llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
>        ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
>      if (I != ExtnameUndeclaredIdentifiers.end()) {
> @@ -7468,7 +7472,11 @@
>      StringLiteral *SE = cast<StringLiteral>(E);
>      NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0), Context,
>                                                  SE->getString(), 0));
> -  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
> +  // Deferred handling of "redefine_extname" pragma.
> +  } else if (!ExtnameUndeclaredIdentifiers.empty() &&
> +             (NewFD->getDeclContext()->isTranslationUnit() ||
> +              NewFD->isExternC()) &&
> +             NewFD->hasExternalFormalLinkage()) {

Do we have a way to express this in slightly less verbose terms? If
not, I think a static helper function would also be reasonable.
Otherwise, LTGTM.

~Aaron

>      llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
>        ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
>      if (I != ExtnameUndeclaredIdentifiers.end()) {
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/