[cfe-commits] r133654 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp test/CodeGen/attr-weak-import.c test/Sema/attr-weak.c

[Douglas Gregor]

On Jun 22, 2011, at 3:08 PM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Wed Jun 22 17:08:50 2011
> New Revision: 133654
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=133654&view=rev
> Log:
> Issue warning if weak_import attribute is added to an already
> declared variable and ignore it. // rdar://9538608
> 
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/test/CodeGen/attr-weak-import.c
>    cfe/trunk/test/Sema/attr-weak.c
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=133654&r1=133653&r2=133654&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jun 22 17:08:50 2011
> @@ -2287,6 +2287,8 @@
>   "inline declaration of %0 not allowed in block scope">;
> def err_static_non_static : Error<
>   "static declaration of %0 follows non-static declaration">;
> +def warn_weak_import : Warning <
> +  "an already-declared variable is made a weak_import declaration %0">;
> def warn_static_non_static : ExtWarn<
>   "static declaration of %0 follows non-static declaration">;
> def err_non_static_static : Error<
> 
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=133654&r1=133653&r2=133654&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Jun 22 17:08:50 2011
> @@ -2039,12 +2039,17 @@
>   }
> 
>   mergeDeclAttributes(New, Old, Context);
> -  // weak_import on current declaration is applied to previous
> -  // tentative definiton.
> +  // Warn if an already-declared variable is made a weak_import in a subsequent declaration
>   if (New->getAttr<WeakImportAttr>() &&
>       Old->getStorageClass() == SC_None &&
> -      !Old->getAttr<WeakImportAttr>())
> -    Old->addAttr(::new (Context) WeakImportAttr(SourceLocation(), Context));
> +      !Old->getAttr<WeakImportAttr>()) {
> +    Diag(New->getLocation(), diag::warn_weak_import) << New->getDeclName();
> +    Diag(Old->getLocation(), diag::note_previous_definition);
> +    // Remove weak_import attribute on new declaration.
> +    // I am just dropping all attributes in curernt decl. We have
> +    // already issued a warning, so we are OK.
> +    New->dropAttrs();
> +  }


We should really drop just the WeakImportAttr, rather than all attributes. Otherwise, we may be breaking other AST invariants that expect attributes to have been propagated from previous declarations to later declarations.

	- Doug