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.ball...@gmail.com> wrote: > On Mon, Jun 29, 2015 at 11:13 AM, Andrey Bokhanko > <andreybokha...@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/ _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
