<div class="__aliyun_email_body_block"><div  style="clear:both;"><span  style="color:#000000;font-size:14.0px;font-family:Tahoma,Arial,STHeiti,SimSun;">Hi Aaron,</span></div><div  style="clear:both;"><span  style="color:#000000;font-size:14.0px;font-family:Tahoma,Arial,STHeiti,SimSun;"><br ></span></div><div  style="clear:both;"><span  style="color:#000000;font-size:14.0px;font-family:Tahoma,Arial,STHeiti,SimSun;">   If I didn't miss anything, the function used in `ASTDumper.cpp` is `noload_lookups` instead of `noload_lookup`. So maybe that wouldn't work.</span></div><div  style="clear:both;"><span  style="color:#000000;font-size:14.0px;font-family:Tahoma,Arial,STHeiti,SimSun;"><br ></span></div><div  style="clear:both;"><span  style="color:#000000;font-size:14.0px;font-family:Tahoma,Arial,STHeiti,SimSun;">   And I checked quickly against the code, it looks like all the existing use of `noload_lookup` has already ensured that the decl context wouldn't be transparent context. So I feel like we can only add unit tests for it if we want.</span></div><div  style="clear:both;"><br ></div><div  style="clear:both;">Thanks,</div><div  style="clear:both;">Chuanqi</div><div  style="clear:both;"><br ></div><blockquote  style="margin-right:.0px;margin-top:.0px;margin-bottom:.0px;font-family:Tahoma,Arial,STHeiti,SimSun;font-size:14.0px;color:#000000;"><div  style="clear:both;">------------------------------------------------------------------</div><div  style="clear:both;">From:Aaron Ballman <aaron@aaronballman.com></div><div  style="clear:both;">Send Time:2024 Apr. 8 (Mon.) 20:20</div><div  style="clear:both;">To:Chuanqi<chuanqi.xcq@alibaba-inc.com></div><div  style="clear:both;">Cc:Chuanqi Xu<yedeng.yd@linux.alibaba.com>; "cfe-commits"<cfe-commits@lists.llvm.org></div><div  style="clear:both;">Subject:Re: [clang] 468dc32 - [NFC] Make `DeclContext::noload_lookup()` accept transparent context</div><div  style="clear:both;"><br ></div>I was thinking we could use the AST dumper to test this via lit<br >because we have `-ast-dump-lookups` which uses `noload_lookup`. Do you<br >think that'd work?<br ><br >~Aaron<br ><br >On Sat, Apr 6, 2024 at 9:59 PM Chuanqi Xu <chuanqi.xcq@alibaba-inc.com> wrote:<br >><br >> Hi Aaron,<br >><br >>     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.<br >><br >>     I can add a unittest if you want. WDYT?<br >><br >> Thanks,<br >> Chuanqi<br >><br >> ------------------------------------------------------------------<br >> From:Aaron Ballman <aaron@aaronballman.com><br >> Send Time:2024 Apr. 3 (Wed.) 19:56<br >> To:Chuanqi Xu<yedeng.yd@linux.alibaba.com><br >> Cc:"cfe-commits"<cfe-commits@lists.llvm.org><br >> Subject:Re: [clang] 468dc32 - [NFC] Make `DeclContext::noload_lookup()` accept transparent context<br >><br >> I'm not opposed to the changes, but they do seem like functional<br >> changes -- we used to trigger an assertion and now we're swallowing it<br >> and changing the behavior; can you add a test case that would have<br >> previously asserted?<br >><br >> ~Aaron<br >><br >> On Wed, Apr 3, 2024 at 3:05 AM Chuanqi Xu via cfe-commits<br >> <cfe-commits@lists.llvm.org> wrote:<br >> ><br >> ><br >> > Author: Chuanqi Xu<br >> > Date: 2024-04-03T15:03:07+08:00<br >> > New Revision: 468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf<br >> ><br >> > URL: <a  href="https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf" target="_blank">https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf</a><br >> > DIFF: <a  href="https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf.diff" target="_blank">https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf.diff</a><br >> ><br >> > LOG: [NFC] Make `DeclContext::noload_lookup()` accept transparent context<br >> ><br >> > Now the `DeclContext::noload_lookup()` asserts that 'this' is not a<br >> > transparent context. However, this is not consistent with<br >> > `DeclContext::lookup()`, which will lookup into its parent context if<br >> > 'this' is a transparent context.<br >> ><br >> > This patch makes the behavior of `DeclContext::noload_lookup()` to be<br >> > consistent with `DeclContext::lookup()`, to lookup into the parent<br >> > context if 'this' is a transparent context.<br >> ><br >> > Added:<br >> ><br >> ><br >> > Modified:<br >> >     clang/lib/AST/DeclBase.cpp<br >> ><br >> > Removed:<br >> ><br >> ><br >> ><br >> > ################################################################################<br >> > diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp<br >> > index 2cbb86b31b5e2e..66a727d9dd0c39 100644<br >> > --- a/clang/lib/AST/DeclBase.cpp<br >> > +++ b/clang/lib/AST/DeclBase.cpp<br >> > @@ -1852,9 +1852,9 @@ DeclContext::lookup(DeclarationName Name) const {<br >> ><br >> >  DeclContext::lookup_result<br >> >  DeclContext::noload_lookup(DeclarationName Name) {<br >> > -  assert(getDeclKind() != Decl::LinkageSpec &&<br >> > -         getDeclKind() != Decl::Export &&<br >> > -         "should not perform lookups into transparent contexts");<br >> > +  // For transparent DeclContext, we should lookup in their enclosing context.<br >> > +  if (getDeclKind() == Decl::LinkageSpec || getDeclKind() == Decl::Export)<br >> > +    return getParent()->noload_lookup(Name);<br >> ><br >> >    DeclContext *PrimaryContext = getPrimaryContext();<br >> >    if (PrimaryContext != this)<br >> ><br >> ><br >> ><br >> > _______________________________________________<br >> > cfe-commits mailing list<br >> > cfe-commits@lists.llvm.org<br >> > <a  href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a></blockquote></div>