[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 19:48:55 PDT 2012
>> 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?
Yeah, sorry for the poor choice of words; should have just said
"removed". I guess my gut tells me that it being just a hack and not
desirable to have in the first place, it would be good to have
guidance for when to remove it.
Kind of like a needle: it's generally not good to shove pieces of
metal under a person's skin, but there are cases where we do it, and
when we do do it, we try to have a plan for getting it out, since it
is not desirable to have in in the first place.
-- Sean Silva
On Thu, Oct 4, 2012 at 9:49 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 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
>
>
More information about the cfe-commits
mailing list