[clang] 468dc32 - [NFC] Make `DeclContext::noload_lookup()` accept transparent context

[Aaron Ballman via cfe-commits]

I was thinking we could use the AST dumper to test this via lit
because we have `-ast-dump-lookups` which uses `noload_lookup`. Do you
think that'd work?

~Aaron

On Sat, Apr 6, 2024 at 9:59 PM Chuanqi Xu <chuanqi.xcq at alibaba-inc.com> wrote:
>
> Hi Aaron,
>
>     This was found in a working in progress local patch that I was using "noload_lookup". Since `noload_lookup` is relatively rarely used in the code base, I assume we can't find a LIT test case that was previously asserted.
>
>     I can add a unittest if you want. WDYT?
>
> Thanks,
> Chuanqi
>
> ------------------------------------------------------------------
> From:Aaron Ballman <aaron at aaronballman.com>
> Send Time:2024 Apr. 3 (Wed.) 19:56
> To:Chuanqi Xu<yedeng.yd at linux.alibaba.com>
> Cc:"cfe-commits"<cfe-commits at lists.llvm.org>
> Subject:Re: [clang] 468dc32 - [NFC] Make `DeclContext::noload_lookup()` accept transparent context
>
> I'm not opposed to the changes, but they do seem like functional
> changes -- we used to trigger an assertion and now we're swallowing it
> and changing the behavior; can you add a test case that would have
> previously asserted?
>
> ~Aaron
>
> On Wed, Apr 3, 2024 at 3:05 AM Chuanqi Xu via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
> >
> >
> > Author: Chuanqi Xu
> > Date: 2024-04-03T15:03:07+08:00
> > New Revision: 468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf
> >
> > URL: https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf
> > DIFF: https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf.diff
> >
> > LOG: [NFC] Make `DeclContext::noload_lookup()` accept transparent context
> >
> > Now the `DeclContext::noload_lookup()` asserts that 'this' is not a
> > transparent context. However, this is not consistent with
> > `DeclContext::lookup()`, which will lookup into its parent context if
> > 'this' is a transparent context.
> >
> > This patch makes the behavior of `DeclContext::noload_lookup()` to be
> > consistent with `DeclContext::lookup()`, to lookup into the parent
> > context if 'this' is a transparent context.
> >
> > Added:
> >
> >
> > Modified:
> >     clang/lib/AST/DeclBase.cpp
> >
> > Removed:
> >
> >
> >
> > ################################################################################
> > diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
> > index 2cbb86b31b5e2e..66a727d9dd0c39 100644
> > --- a/clang/lib/AST/DeclBase.cpp
> > +++ b/clang/lib/AST/DeclBase.cpp
> > @@ -1852,9 +1852,9 @@ DeclContext::lookup(DeclarationName Name) const {
> >
> >  DeclContext::lookup_result
> >  DeclContext::noload_lookup(DeclarationName Name) {
> > -  assert(getDeclKind() != Decl::LinkageSpec &&
> > -         getDeclKind() != Decl::Export &&
> > -         "should not perform lookups into transparent contexts");
> > +  // For transparent DeclContext, we should lookup in their enclosing context.
> > +  if (getDeclKind() == Decl::LinkageSpec || getDeclKind() == Decl::Export)
> > +    return getParent()->noload_lookup(Name);
> >
> >    DeclContext *PrimaryContext = getPrimaryContext();
> >    if (PrimaryContext != this)
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits