r208659 - Refactor and fix a latent bug (found by inspection) where an external AST

[Richard Smith]

It's not that; the ASTReader doesn't register these hooks.

I've hit something that looks exactly like PR19061 modularizing LLVM. I
have a hack around it, but not a proper fix yet.
On 12 May 2014 22:36, "Jordan Rose" <jordan_rose at apple.com> wrote:

> I wonder if this is http://llvm.org/bugs/show_bug.cgi?id=19061. (I don't
> have a clean tree to rebuild/test with right now.)
>
> Jordan
>
>
> On May 12, 2014, at 17:34 , Richard Smith <richard-llvm at metafoo.co.uk>
> wrote:
>
> > Author: rsmith
> > Date: Mon May 12 19:34:43 2014
> > New Revision: 208659
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=208659&view=rev
> > Log:
> > Refactor and fix a latent bug (found by inspection) where an external AST
> > source that provides a declaration from a hidden module would not have
> the
> > visibility of the produced definition checked. This might matter if an
> > external source chose to import a new module to provide an extra
> definition,
> > but is not observable with our current external sources.
> >
> > Modified:
> >    cfe/trunk/lib/Sema/SemaType.cpp
> >
> > Modified: cfe/trunk/lib/Sema/SemaType.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=208659&r1=208658&r2=208659&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaType.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaType.cpp Mon May 12 19:34:43 2014
> > @@ -5214,39 +5214,36 @@ bool Sema::RequireCompleteTypeImpl(Sourc
> >     return false;
> >   }
> >
> > -  // FIXME: If there's an unimported definition of this type in a
> module (for
> > +  const TagType *Tag = T->getAs<TagType>();
> > +  const ObjCInterfaceType *IFace = T->getAs<ObjCInterfaceType>();
> > +
> > +  // If there's an unimported definition of this type in a module (for
> >   // instance, because we forward declared it, then imported the
> definition),
> >   // import that definition now.
> > +  //
> >   // FIXME: What about other cases where an import extends a
> redeclaration
> >   // chain for a declaration that can be accessed through a mechanism
> other
> >   // than name lookup (eg, referenced in a template, or a variable whose
> type
> >   // could be completed by the module)?
> > +  if (Tag || IFace) {
> > +    NamedDecl *D =
> > +        Tag ? static_cast<NamedDecl *>(Tag->getDecl()) :
> IFace->getDecl();
> >
> > -  const TagType *Tag = T->getAs<TagType>();
> > -  const ObjCInterfaceType *IFace = 0;
> > -
> > -  if (Tag) {
> > -    // Avoid diagnosing invalid decls as incomplete.
> > -    if (Tag->getDecl()->isInvalidDecl())
> > -      return true;
> > -
> > -    // Give the external AST source a chance to complete the type.
> > -    if (Tag->getDecl()->hasExternalLexicalStorage()) {
> > -      Context.getExternalSource()->CompleteType(Tag->getDecl());
> > -      if (!Tag->isIncompleteType())
> > -        return false;
> > -    }
> > -  }
> > -  else if ((IFace = T->getAs<ObjCInterfaceType>())) {
> >     // Avoid diagnosing invalid decls as incomplete.
> > -    if (IFace->getDecl()->isInvalidDecl())
> > +    if (D->isInvalidDecl())
> >       return true;
> >
> >     // Give the external AST source a chance to complete the type.
> > -    if (IFace->getDecl()->hasExternalLexicalStorage()) {
> > -      Context.getExternalSource()->CompleteType(IFace->getDecl());
> > -      if (!IFace->isIncompleteType())
> > -        return false;
> > +    if (auto *Source = Context.getExternalSource()) {
> > +      if (Tag)
> > +        Source->CompleteType(Tag->getDecl());
> > +      else
> > +        Source->CompleteType(IFace->getDecl());
> > +
> > +      // If the external source completed the type, go through the
> motions
> > +      // again to ensure we're allowed to use the completed type.
> > +      if (!T->isIncompleteType())
> > +        return RequireCompleteTypeImpl(Loc, T, Diagnoser);
> >     }
> >   }
> >
> >
> >
> > _______________________________________________
> > 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/20140512/423c77ac/attachment.html>