fix for assertion fail for pragma weak on typedef

[amusman]

Summary

Can you please review a fix for assertion fail on the following small test:

typedef int td3;
#pragma weak td3 = td3

Assertion failed: (isa<FunctionDecl>(ND) || isa<VarDecl>(ND)), function DeclClonePragmaWeak, file /export/bpart/users/amusman/workspaces/blib16/llvm/tools/clang/lib/Sema/SemaDeclAttr.cpp, line 5337.

Call stack:

frame #4: 0x0000000103e2eb61 clang`__assert_rtn(func=0x0000000103f0dec0, file=0x0000000103f0dd75, line=5337, expr=0x0000000103f0ded4) + 129 at Signals.inc:537
frame #5: 0x0000000100d6b5da clang`clang::Sema::DeclClonePragmaWeak(this=0x000000010a051a00, ND=0x000000010a030810, II=0x000000010a0720d0, Loc=SourceLocation at 0x00007fff5fbf93d0) + 170 at SemaDeclAttr.cpp:5337
frame #6: 0x0000000100d6bbd5 clang`clang::Sema::DeclApplyPragmaWeak(this=0x000000010a051a00, S=0x0000000109b112f0, ND=0x000000010a030810, W=0x00007fff5fbf95e8) + 181 at SemaDeclAttr.cpp:5388
frame #7: 0x0000000100d43397 clang`clang::Sema::ActOnPragmaWeakAlias(this=0x000000010a051a00, Name=0x000000010a0720d0, AliasName=0x000000010a072068, PragmaLoc=SourceLocation at 0x00007fff5fbf9638, NameLoc=SourceLocation at 0x00007fff5fbf9630, AliasNameLoc=SourceLocation at 0x00007fff5fbf9628) + 295 at SemaDecl.cpp:14221
frame #8: 0x000000010098e6d5 clang`clang::Parser::HandlePragmaWeakAlias(this=0x000000010a053a00) + 261 at ParsePragma.cpp:433
frame #9: 0x00000001009c152e clang`clang::Parser::ParseExternalDeclaration(this=0x000000010a053a00, attrs=0x00007fff5fbf99d8, DS=0x0000000000000000) + 1790 at Parser.cpp:702
frame #10: 0x00000001009c0de7 clang`clang::Parser::ParseTopLevelDecl(this=0x000000010a053a00, Result=0x00007fff5fbf9b00) + 743 at Parser.cpp:572
frame #11: 0x000000010090d45d clang`clang::ParseAST(S=0x000000010a051a00, PrintStats=false, SkipFunctionBodies=false) + 989 at ParseAST.cpp:144
frame #12: 0x000000010017262f clang`clang::ASTFrontendAction::ExecuteAction(this=0x0000000109b090a0) + 511 at FrontendAction.cpp:537
frame #13: 0x0000000100171bc0 clang`clang::FrontendAction::Execute(this=0x0000000109b090a0) + 112 at FrontendAction.cpp:439
frame #14: 0x0000000100101865 clang`clang::CompilerInstance::ExecuteAction(this=0x0000000109b084e0, Act=0x0000000109b090a0) + 997 at CompilerInstance.cpp:806
frame #15: 0x0000000100020cbe clang`clang::ExecuteCompilerInvocation(Clang=0x0000000109b084e0) + 3246 at ExecuteCompilerInvocation.cpp:318
frame #16: 0x0000000100001449 clang`cc1_main(Argv=ArrayRef<const char *> at 0x00007fff5fbfa6a8, Argv0=0x00007fff5fbfd1b8, MainAddr=0x00000001000149f0) + 2473 at cc1_main.cpp:110
frame #17: 0x0000000100016133 clang`ExecuteCC1Tool(argv=ArrayRef<const char *> at 0x00007fff5fbfb168, Tool=StringRef at 0x00007fff5fbfb158) + 163 at driver.cpp:369
frame #18: 0x0000000100014fb5 clang`main(argc_=3, argv_=0x00007fff5fbfc9c8) + 1269 at driver.cpp:415

[aaron.ballman]

Thanks for this!

test/CodeGen/pragma-weak.c

63 ↗

(On Diff #34888)

This diagnostic isn't useful to the user; __td3 was declared as a typedef declaration. I would prefer something a bit more user-friendly; ideally using diag::warn_attribute_wrong_decl_type.

[amusman]

Hi Aaron,

Thank you for review. I've updated the warning for such cases.

Regards,
Alexander

[aaron.ballman]

LGTM with one minor nit.

lib/Sema/Sema.cpp

729 ↗

(On Diff #34987)

Should be "'weak'" instead of "weak" so that the diagnostic is properly quoted for the user.

cfe/trunk/lib/Sema/Sema.cpp

void Sema::ActOnEndOfTranslationUnit() {
   }
 
   // Check for #pragma weak identifiers that were never declared
   LoadExternalWeakUndeclaredIdentifiers();
   for (auto WeakID : WeakUndeclaredIdentifiers) {
     if (WeakID.second.getUsed())
       continue;
 
+    Decl *PrevDecl = LookupSingleName(TUScope, WeakID.first, SourceLocation(),
+                                      LookupOrdinaryName);
+    if (PrevDecl != nullptr &&
+        !(isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl)))
+      Diag(WeakID.second.getLocation(), diag::warn_attribute_wrong_decl_type)
+          << "'weak'" << ExpectedVariableOrFunction;
+    else
       Diag(WeakID.second.getLocation(), diag::warn_weak_identifier_undeclared)
           << WeakID.first;
   }
 
   if (LangOpts.CPlusPlus11 &&
       !Diags.isIgnored(diag::warn_delegating_ctor_cycle, SourceLocation()))
     CheckDelegatingCtorCycles();
 
   if (TUKind == TU_Module) {
     // If we are building a module, resolve all of the exported declarations

cfe/trunk/lib/Sema/SemaDecl.cpp

void Sema::ActOnPragmaWeakAlias(IdentifierInfo* Name,
                                 IdentifierInfo* AliasName,
                                 SourceLocation PragmaLoc,
                                 SourceLocation NameLoc,
                                 SourceLocation AliasNameLoc) {
   Decl *PrevDecl = LookupSingleName(TUScope, AliasName, AliasNameLoc,
                                     LookupOrdinaryName);
   WeakInfo W = WeakInfo(Name, NameLoc);
 
-  if (PrevDecl) {
+  if (PrevDecl && (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl))) {
     if (!PrevDecl->hasAttr<AliasAttr>())
       if (NamedDecl *ND = dyn_cast<NamedDecl>(PrevDecl))
         DeclApplyPragmaWeak(TUScope, ND, W);
   } else {
     (void)WeakUndeclaredIdentifiers.insert(
       std::pair<IdentifierInfo*,WeakInfo>(AliasName, W));
   }
 }

cfe/trunk/test/CodeGen/pragma-weak.c

 // CHECK-LABEL: define void @__foo2()
 
 
 ///// test errors
 
 #pragma weak unused // expected-warning {{weak identifier 'unused' never declared}}
 #pragma weak unused_alias = __unused_alias  // expected-warning {{weak identifier '__unused_alias' never declared}}
 
-#pragma weak td // expected-warning {{weak identifier 'td' never declared}}
+#pragma weak td // expected-warning {{'weak' attribute only applies to variables and functions}}
 typedef int td;
 
-#pragma weak td2 = __td2 // expected-warning {{weak identifier '__td2' never declared}}
+#pragma weak td2 = __td2 // expected-warning {{'weak' attribute only applies to variables and functions}}
 typedef int __td2;
 
+typedef int __td3;
+#pragma weak td3 = __td3 // expected-warning {{'weak' attribute only applies to variables and functions}}
 
 ///// test weird cases
 
 // test repeats
 
 #pragma weak stutter = __stutter
 #pragma weak stutter = __stutter
 void __stutter(void) {}