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

[Jordan Rose]

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