[cfe-commits] r165263 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclCXX.cpp test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp test/SemaCXX/libstdcxx_atomic_ns_hack.cpp
Sean Silva
silvas at purdue.edu
Thu Oct 4 18:20:16 PDT 2012
+ if (*IsInline && II && II->getName().startswith("__atomic") &&
+ S.getSourceManager().isInSystemHeader(Loc)) {
+ // libstdc++4.6's <atomic> reopens a non-inline namespace as inline, and
+ // expects that to affect the earlier declaration.
+ for (NamespaceDecl *NS = PrevNS->getMostRecentDecl(); NS;
+ NS = NS->getPreviousDecl())
+ NS->setInline(*IsInline);
+ // Patch up the lookup table for the containing namespace. This
isn't really
+ // correct, but it's good enough for this particular case.
Could you call out the fact that this is a hack a bit more loudly? It
all seems quite casual with the short explanation tucked away nice and
tidy inside the `if`. Something like `// HACK: ....` above the `if`?
Also, it's probably worth saying under what conditions the hack should
be pulled. Otherwise there's a risk that it will get pulled
inappropriately.
btw, I love the #ifdef BE_THE_HEADER thing :)
-- Sean Silva
On Thu, Oct 4, 2012 at 6:13 PM, Richard Smith
<richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Thu Oct 4 17:13:39 2012
> New Revision: 165263
>
> URL: http://llvm.org/viewvc/llvm-project?rev=165263&view=rev
> Log:
> Egriegious hack to support libstdc++4.6's broken <atomic> header, which defines
> a non-inline namespace, then reopens it as inline to try to add its symbols to
> the surrounding namespace. In this one special case, permit the namespace to be
> reopened as inline, and patch up the name lookup tables to match.
>
> Added:
> cfe/trunk/test/SemaCXX/libstdcxx_atomic_ns_hack.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=165263&r1=165262&r2=165263&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 4 17:13:39 2012
> @@ -781,7 +781,7 @@
> def err_static_assert_failed : Error<"static_assert failed %0">;
>
> def warn_inline_namespace_reopened_noninline : Warning<
> - "inline namespace cannot be re-opened as a non-inline namespace">;
> + "inline namespace cannot be reopened as a non-inline namespace">;
> def err_inline_namespace_mismatch : Error<
> "%select{|non-}0inline namespace "
> "cannot be reopened as %select{non-|}0inline">;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=165263&r1=165262&r2=165263&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Oct 4 17:13:39 2012
> @@ -5399,7 +5399,42 @@
> // Namespace Handling
> //===----------------------------------------------------------------------===//
>
> +/// \brief Diagnose a mismatch in 'inline' qualifiers when a namespace is
> +/// reopened.
> +static void DiagnoseNamespaceInlineMismatch(Sema &S, SourceLocation KeywordLoc,
> + SourceLocation Loc,
> + IdentifierInfo *II, bool *IsInline,
> + NamespaceDecl *PrevNS) {
> + assert(*IsInline != PrevNS->isInline());
> +
> + if (*IsInline && II && II->getName().startswith("__atomic") &&
> + S.getSourceManager().isInSystemHeader(Loc)) {
> + // libstdc++4.6's <atomic> reopens a non-inline namespace as inline, and
> + // expects that to affect the earlier declaration.
> + for (NamespaceDecl *NS = PrevNS->getMostRecentDecl(); NS;
> + NS = NS->getPreviousDecl())
> + NS->setInline(*IsInline);
> + // Patch up the lookup table for the containing namespace. This isn't really
> + // correct, but it's good enough for this particular case.
> + for (DeclContext::decl_iterator I = PrevNS->decls_begin(),
> + E = PrevNS->decls_end(); I != E; ++I)
> + if (NamedDecl *ND = dyn_cast<NamedDecl>(*I))
> + PrevNS->getParent()->makeDeclVisibleInContext(ND);
> + return;
> + }
>
> + if (PrevNS->isInline())
> + // The user probably just forgot the 'inline', so suggest that it
> + // be added back.
> + S.Diag(Loc, diag::warn_inline_namespace_reopened_noninline)
> + << FixItHint::CreateInsertion(KeywordLoc, "inline ");
> + else
> + S.Diag(Loc, diag::err_inline_namespace_mismatch)
> + << IsInline;
> +
> + S.Diag(PrevNS->getLocation(), diag::note_previous_definition);
> + *IsInline = PrevNS->isInline();
> +}
>
> /// ActOnStartNamespaceDef - This is called at the start of a namespace
> /// definition.
> @@ -5449,21 +5484,9 @@
>
> if (PrevNS) {
> // This is an extended namespace definition.
> - if (IsInline != PrevNS->isInline()) {
> - // inline-ness must match
> - if (PrevNS->isInline()) {
> - // The user probably just forgot the 'inline', so suggest that it
> - // be added back.
> - Diag(Loc, diag::warn_inline_namespace_reopened_noninline)
> - << FixItHint::CreateInsertion(NamespaceLoc, "inline ");
> - } else {
> - Diag(Loc, diag::err_inline_namespace_mismatch)
> - << IsInline;
> - }
> - Diag(PrevNS->getLocation(), diag::note_previous_definition);
> -
> - IsInline = PrevNS->isInline();
> - }
> + if (IsInline != PrevNS->isInline())
> + DiagnoseNamespaceInlineMismatch(*this, NamespaceLoc, Loc, II,
> + &IsInline, PrevNS);
> } else if (PrevDecl) {
> // This is an invalid name redefinition.
> Diag(Loc, diag::err_redefinition_different_kind)
> @@ -5494,15 +5517,9 @@
> PrevNS = ND->getAnonymousNamespace();
> }
>
> - if (PrevNS && IsInline != PrevNS->isInline()) {
> - // inline-ness must match
> - Diag(Loc, diag::err_inline_namespace_mismatch)
> - << IsInline;
> - Diag(PrevNS->getLocation(), diag::note_previous_definition);
> -
> - // Recover by ignoring the new namespace's inline status.
> - IsInline = PrevNS->isInline();
> - }
> + if (PrevNS && IsInline != PrevNS->isInline())
> + DiagnoseNamespaceInlineMismatch(*this, NamespaceLoc, NamespaceLoc, II,
> + &IsInline, PrevNS);
> }
>
> NamespaceDecl *Namespc = NamespaceDecl::Create(Context, CurContext, IsInline,
>
> Modified: cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp?rev=165263&r1=165262&r2=165263&view=diff
> ==============================================================================
> --- cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp (original)
> +++ cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.def/p7.cpp Thu Oct 4 17:13:39 2012
> @@ -3,11 +3,11 @@
> namespace NIL {} // expected-note {{previous definition}}
> inline namespace NIL {} // expected-error {{cannot be reopened as inline}}
> inline namespace IL {} // expected-note {{previous definition}}
> -namespace IL {} // expected-warning{{inline namespace cannot be re-opened as a non-inline namespace}}
> +namespace IL {} // expected-warning{{inline namespace cannot be reopened as a non-inline namespace}}
>
> namespace {} // expected-note {{previous definition}}
> inline namespace {} // expected-error {{cannot be reopened as inline}}
> namespace X {
> inline namespace {} // expected-note {{previous definition}}
> - namespace {} // expected-error {{cannot be reopened as non-inline}}
> + namespace {} // expected-warning {{cannot be reopened as a non-inline namespace}}
> }
>
> Added: cfe/trunk/test/SemaCXX/libstdcxx_atomic_ns_hack.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/libstdcxx_atomic_ns_hack.cpp?rev=165263&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/libstdcxx_atomic_ns_hack.cpp (added)
> +++ cfe/trunk/test/SemaCXX/libstdcxx_atomic_ns_hack.cpp Thu Oct 4 17:13:39 2012
> @@ -0,0 +1,35 @@
> +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
> +
> +// libstdc++ 4.6.x contains a bug where it defines std::__atomic[0,1,2] as a
> +// non-inline namespace, then selects one of those namespaces and reopens it
> +// as inline, as a strange way of providing something like a using-directive.
> +// Clang has an egregious hack to work around the problem, by allowing a
> +// namespace to be converted from non-inline to inline in this one specific
> +// case.
> +
> +#ifdef BE_THE_HEADER
> +
> +#pragma clang system_header
> +
> +namespace std {
> + namespace __atomic0 {
> + typedef int foobar;
> + }
> + namespace __atomic1 {
> + typedef void foobar;
> + }
> +
> + inline namespace __atomic0 {}
> +}
> +
> +#else
> +
> +#define BE_THE_HEADER
> +#include "libstdcxx_atomic_ns_hack.cpp"
> +
> +std::foobar fb;
> +
> +using T = void; // expected-note {{here}}
> +using T = std::foobar; // expected-error {{different types ('std::foobar' (aka 'int') vs 'void')}}
> +
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list