r280728 - Modules: Fix an assertion in DeclContext::buildLookup.

[Richard Smith via cfe-commits]

On Tue, Sep 6, 2016 at 11:16 AM, Manman Ren via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: mren
> Date: Tue Sep  6 13:16:54 2016
> New Revision: 280728
>
> URL: http://llvm.org/viewvc/llvm-project?rev=280728&view=rev
> Log:
> Modules: Fix an assertion in DeclContext::buildLookup.
>
> When calling getMostRecentDecl, we can pull in more definitions from
> a module. We call getPrimaryContext afterwards to make sure that
> we buildLookup on a primary context.
>
> rdar://27926200
>
> Added:
>     cfe/trunk/test/Modules/Inputs/lookup-assert/
>     cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h
>     cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h
>     cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h
>     cfe/trunk/test/Modules/Inputs/lookup-assert/module.map
>     cfe/trunk/test/Modules/lookup-assert.m
> Modified:
>     cfe/trunk/lib/AST/DeclBase.cpp
>
> Modified: cfe/trunk/lib/AST/DeclBase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> DeclBase.cpp?rev=280728&r1=280727&r2=280728&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/AST/DeclBase.cpp (original)
> +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Sep  6 13:16:54 2016
> @@ -1411,10 +1411,6 @@ DeclContext::lookup(DeclarationName Name
>    assert(DeclKind != Decl::LinkageSpec &&
>           "Should not perform lookups into linkage specs!");
>
> -  const DeclContext *PrimaryContext = getPrimaryContext();
> -  if (PrimaryContext != this)
> -    return PrimaryContext->lookup(Name);
> -
>    // If we have an external source, ensure that any later redeclarations
> of this
>    // context have been loaded, since they may add names to the result of
> this
>    // lookup (or add external visible storage).
> @@ -1422,6 +1418,12 @@ DeclContext::lookup(DeclarationName Name
>    if (Source)
>      (void)cast<Decl>(this)->getMostRecentDecl();
>
> +  // getMostRecentDecl can change the result of getPrimaryContext. Call
> +  // getPrimaryContext afterwards.
> +  const DeclContext *PrimaryContext = getPrimaryContext();
> +  if (PrimaryContext != this)
> +    return PrimaryContext->lookup(Name);
>

This seems like a bug in getPrimaryContext() -- if there is a
not-yet-loaded definition of the DeclContext, getPrimaryContext() should
return that instead of returning a non-defining declaration. Why is
ObjCInterfaceDecl::hasDefinition (indirectly called by getPrimaryContext)
not loading the definition in this case?


> +
>    if (hasExternalVisibleStorage()) {
>      assert(Source && "external visible storage but no external source?");
>
>
> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/Inputs/lookup-assert/Base.h?rev=280728&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h (added)
> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h Tue Sep  6
> 13:16:54 2016
> @@ -0,0 +1,4 @@
> + at interface BaseInterface
> +- (void) test;
> + at end
> +
>
> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/Inputs/lookup-assert/Derive.h?rev=280728&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h (added)
> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h Tue Sep  6
> 13:16:54 2016
> @@ -0,0 +1,3 @@
> +#include "Base.h"
> + at interface DerivedInterface : BaseInterface
> + at end
>
> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/Inputs/lookup-assert/H3.h?rev=280728&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h (added)
> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h Tue Sep  6 13:16:54
> 2016
> @@ -0,0 +1 @@
> +#include "Base.h"
>
> Added: cfe/trunk/test/Modules/Inputs/lookup-assert/module.map
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/Inputs/lookup-assert/module.map?rev=280728&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/Inputs/lookup-assert/module.map (added)
> +++ cfe/trunk/test/Modules/Inputs/lookup-assert/module.map Tue Sep  6
> 13:16:54 2016
> @@ -0,0 +1,4 @@
> +module X {
> +  header "H3.h"
> +  export *
> +}
>
> Added: cfe/trunk/test/Modules/lookup-assert.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/lookup-assert.m?rev=280728&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/lookup-assert.m (added)
> +++ cfe/trunk/test/Modules/lookup-assert.m Tue Sep  6 13:16:54 2016
> @@ -0,0 +1,10 @@
> +// RUN: rm -rf %t
> +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules
> -fimplicit-module-maps -I %S/Inputs/lookup-assert %s -verify
> +// expected-no-diagnostics
> +
> +#include "Derive.h"
> +#import <H3.h>
> + at implementation DerivedInterface
> +- (void)test {
> +}
> + at end
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160906/ef1b40ac/attachment.html>