[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 11:58 AM, Andrey Bokhanko
<andreybokhanko at gmail.com> wrote:
> Patch updated in response to Aaron Ballman's comments (http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20150622/131513.html).
>
>
> http://reviews.llvm.org/D10646
>
> Files:
>   include/clang/Sema/Sema.h
>   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: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -7508,6 +7508,10 @@
>                                    SourceLocation WeakNameLoc,
>                                    SourceLocation AliasNameLoc);
>
> +  /// HandleDeferredPragmaRedefineExtname - Deferred handling of
> +  // '\#pragma redefine_extname' for given declration.

Typo: declration

> +  template <typename T> void HandleDeferredPragmaRedefineExtname(T *Decl);

I wasn't envisioning something like this. I was thinking something more like:

bool Decl::isExternallyAvailable() const; // Probably a terrible name

That then gets called from SemaDecl.cpp.

If that API is too specific for a Decl, then it would make more sense
as a helper function in SemaDecl.cpp. Also, no need for it to be a
templated function if we go this route.

~Aaron

> +
>    /// ActOnPragmaWeakAlias - Called on well formed \#pragma weak ident = ident.
>    void ActOnPragmaWeakAlias(IdentifierInfo* WeakName,
>                              IdentifierInfo* AliasName,
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp
> +++ lib/Sema/SemaDecl.cpp
> @@ -5946,14 +5946,9 @@
>
>      NewVD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0),
>                                                  Context, Label, 0));
> -  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
> -    llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
> -      ExtnameUndeclaredIdentifiers.find(NewVD->getIdentifier());
> -    if (I != ExtnameUndeclaredIdentifiers.end()) {
> -      NewVD->addAttr(I->second);
> -      ExtnameUndeclaredIdentifiers.erase(I);
> -    }
> -  }
> +  // Deferred handling of "redefine_extname" pragma.
> +  } else
> +    HandleDeferredPragmaRedefineExtname(NewVD);
>
>    // Diagnose shadowed variables before filtering for scope.
>    if (D.getCXXScopeSpec().isEmpty())
> @@ -7468,14 +7463,9 @@
>      StringLiteral *SE = cast<StringLiteral>(E);
>      NewFD->addAttr(::new (Context) AsmLabelAttr(SE->getStrTokenLoc(0), Context,
>                                                  SE->getString(), 0));
> -  } else if (!ExtnameUndeclaredIdentifiers.empty()) {
> -    llvm::DenseMap<IdentifierInfo*,AsmLabelAttr*>::iterator I =
> -      ExtnameUndeclaredIdentifiers.find(NewFD->getIdentifier());
> -    if (I != ExtnameUndeclaredIdentifiers.end()) {
> -      NewFD->addAttr(I->second);
> -      ExtnameUndeclaredIdentifiers.erase(I);
> -    }
> -  }
> +  // Deferred handling of "redefine_extname" pragma.
> +  } else
> +    HandleDeferredPragmaRedefineExtname(NewFD);
>
>    // Copy the parameter declarations from the declarator D to the function
>    // declaration NewFD, if they are available.  First scavenge them into Params.
> @@ -14266,6 +14256,24 @@
>      (void)ExtnameUndeclaredIdentifiers.insert(std::make_pair(Name, Attr));
>  }
>
> +template<typename T>
> +void Sema::HandleDeferredPragmaRedefineExtname(T *Decl) {
> +  if (ExtnameUndeclaredIdentifiers.empty())
> +    return;
> +
> +  // Declaration is not suitable for pragma redefine_extname
> +  if (!(Decl->getDeclContext()->isTranslationUnit() || Decl->isExternC()) ||
> +      !Decl->hasExternalFormalLinkage())
> +    return;
> +
> +  llvm::DenseMap<IdentifierInfo *, AsmLabelAttr *>::iterator I =
> +      ExtnameUndeclaredIdentifiers.find(Decl->getIdentifier());
> +  if (I != ExtnameUndeclaredIdentifiers.end()) {
> +    Decl->addAttr(I->second);
> +    ExtnameUndeclaredIdentifiers.erase(I);
> +  }
> +}
> +
>  void Sema::ActOnPragmaWeakID(IdentifierInfo* Name,
>                               SourceLocation PragmaLoc,
>                               SourceLocation NameLoc) {
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/