[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
Richard Smith
richard at metafoo.co.uk
Thu Oct 4 18:49:34 PDT 2012
On Thu, Oct 4, 2012 at 6:20 PM, Sean Silva <silvas at purdue.edu> wrote:
> + 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`?
>
Sounds sensible to me. r165286.
> 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.
>
What do you mean by pulled? Dropped?
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121004/a4c6486a/attachment.html>
More information about the cfe-commits
mailing list