r359814 - [Sema] Emit warning for visibility attribute on internal-linkage declaration

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu May 2 18:42:40 PDT 2019


This will trigger computation and caching the linkage of the
declaration too early (before we actually know it) in some cases. For
instance, I believe this is causing a rejects-valid on:

typedef struct __attribute__((visibility("hidden"))) {} A;

... which we used to accept but now reject with:

<stdin>:1:31: warning: 'visibility' attribute is ignored on a
non-external symbol [-Wignored-attributes]
typedef struct __attribute__((visibility("hidden"))) {} A;
                              ^
<stdin>:1:57: error: unsupported: typedef changes linkage of anonymous
type, but linkage was already computed
typedef struct __attribute__((visibility("hidden"))) {} A;
                                                        ^
<stdin>:1:15: note: use a tag name here to establish linkage prior to definition
typedef struct __attribute__((visibility("hidden"))) {} A;
              ^
               A

In addition to the error, note that the warning is wrong, because the
linkage was computed too early (before it was known).

On Thu, 2 May 2019 at 12:01, Scott Linder via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: scott.linder
> Date: Thu May  2 12:03:57 2019
> New Revision: 359814
>
> URL: http://llvm.org/viewvc/llvm-project?rev=359814&view=rev
> Log:
> [Sema] Emit warning for visibility attribute on internal-linkage declaration
>
> GCC warns on these cases, but we currently just silently ignore the attribute.
>
> Differential Revision: https://reviews.llvm.org/D61097
>
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>     cfe/trunk/test/Sema/attr-visibility.c
>     cfe/trunk/test/SemaCXX/ast-print.cpp
>     cfe/trunk/test/SemaCXX/attr-visibility.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=359814&r1=359813&r2=359814&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu May  2 12:03:57 2019
> @@ -2778,6 +2778,9 @@ def warn_attribute_ignored : Warning<"%0
>  def warn_attribute_ignored_on_inline :
>    Warning<"%0 attribute ignored on inline function">,
>    InGroup<IgnoredAttributes>;
> +def warn_attribute_ignored_on_non_external :
> +  Warning<"%0 attribute is ignored on a non-external symbol">,
> +  InGroup<IgnoredAttributes>;
>  def warn_nocf_check_attribute_ignored :
>    Warning<"'nocf_check' attribute ignored; use -fcf-protection to enable the attribute">,
>    InGroup<IgnoredAttributes>;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=359814&r1=359813&r2=359814&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu May  2 12:03:57 2019
> @@ -2615,6 +2615,14 @@ static void handleVisibilityAttr(Sema &S
>      return;
>    }
>
> +  // Visibility attributes have no effect on symbols with internal linkage.
> +  if (const auto *ND = dyn_cast<NamedDecl>(D)) {
> +    if (!ND->isExternallyVisible())
> +      S.Diag(AL.getRange().getBegin(),
> +             diag::warn_attribute_ignored_on_non_external)
> +          << AL;
> +  }
> +
>    // Check that the argument is a string literal.
>    StringRef TypeStr;
>    SourceLocation LiteralLoc;
>
> Modified: cfe/trunk/test/Sema/attr-visibility.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-visibility.c?rev=359814&r1=359813&r2=359814&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/attr-visibility.c (original)
> +++ cfe/trunk/test/Sema/attr-visibility.c Thu May  2 12:03:57 2019
> @@ -26,3 +26,9 @@ typedef int __attribute__((visibility("d
>  int x __attribute__((type_visibility("default"))); // expected-error {{'type_visibility' attribute only applies to types and namespaces}}
>
>  int PR17105 __attribute__((visibility(hidden))); // expected-error {{'visibility' attribute requires a string}}
> +
> +static int test8 __attribute__((visibility("default"))); // expected-warning {{'visibility' attribute is ignored on a non-external symbol}}
> +static int test9 __attribute__((visibility("hidden"))); // expected-warning {{'visibility' attribute is ignored on a non-external symbol}}
> +static int test10 __attribute__((visibility("internal"))); // expected-warning {{'visibility' attribute is ignored on a non-external symbol}}
> +
> +static int test11() __attribute__((visibility("default"))); // expected-warning {{'visibility' attribute is ignored on a non-external symbol}}
>
> Modified: cfe/trunk/test/SemaCXX/ast-print.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ast-print.cpp?rev=359814&r1=359813&r2=359814&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/ast-print.cpp (original)
> +++ cfe/trunk/test/SemaCXX/ast-print.cpp Thu May  2 12:03:57 2019
> @@ -209,10 +209,8 @@ void test(int i) {
>  }
>  }
>
> -namespace {
>  // CHECK: struct {{\[\[gnu::visibility\(\"hidden\"\)\]\]}} S;
>  struct [[gnu::visibility("hidden")]] S;
> -}
>
>  // CHECK:      struct CXXFunctionalCastExprPrint {
>  // CHECK-NEXT: } fce = CXXFunctionalCastExprPrint{};
>
> Modified: cfe/trunk/test/SemaCXX/attr-visibility.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-visibility.cpp?rev=359814&r1=359813&r2=359814&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/attr-visibility.cpp (original)
> +++ cfe/trunk/test/SemaCXX/attr-visibility.cpp Thu May  2 12:03:57 2019
> @@ -18,3 +18,9 @@ void foo<int>() {
>  struct x3 {
>    static int y;
>  } __attribute((visibility("default"))); // expected-warning {{attribute 'visibility' after definition is ignored}}
> +
> +const int test4 __attribute__((visibility("default"))) = 0; // expected-warning {{'visibility' attribute is ignored on a non-external symbol}}
> +
> +namespace {
> +  int test5 __attribute__((visibility("default"))); // expected-warning {{'visibility' attribute is ignored on a non-external symbol}}
> +};
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list