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

[Chuanqi Xu via cfe-commits]

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 <https://github.com/llvm/llvm-project/commit/468dc32ff55d19f55132cbcc4d6ceb1f6d1c12cf >
> DIFF: 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 <https://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/20240407/50bbbde8/attachment.html>