https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/172430
Close https://github.com/llvm/llvm-project/issues/172241 This was meant to be a fix. Just during my debugging, I want to see the effect/reason/motivation of the action, then I simply remove the loop to demote var definition as declaration, and all tests suddenly passed. I've also tested our internal workloads and it looks well too. So I am wondering if this can be a real fix. The comment says “We should keep at most one definition on the chain.” But I am not sure if this is a strong restriction. To my mind model, I think it is fine as long as the definitions along the chain are equal. @zygoloid would you like to take a look? Why do we have this and do we have a counter example? @mpark @alexfh maybe you're interested to test this against your internal workloads to avoid break your internal code later. >From 4eb4dcbb848523ffc323fcf9a6b2bbce4036232f Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Tue, 16 Dec 2025 14:12:41 +0800 Subject: [PATCH] Stop demote var definition as declaration --- clang/lib/Serialization/ASTReaderDecl.cpp | 13 ------- clang/test/Modules/pr172241.cppm | 47 +++++++++++++++++++++++ 2 files changed, 47 insertions(+), 13 deletions(-) create mode 100644 clang/test/Modules/pr172241.cppm diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index d245a111088d5..f1b7a3c937551 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -3645,19 +3645,6 @@ void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, auto *PrevVD = cast<VarDecl>(Previous); D->RedeclLink.setPrevious(PrevVD); D->First = PrevVD->First; - - // We should keep at most one definition on the chain. - // FIXME: Cache the definition once we've found it. Building a chain with - // N definitions currently takes O(N^2) time here. - if (VD->isThisDeclarationADefinition() == VarDecl::Definition) { - for (VarDecl *CurD = PrevVD; CurD; CurD = CurD->getPreviousDecl()) { - if (CurD->isThisDeclarationADefinition() == VarDecl::Definition) { - Reader.mergeDefinitionVisibility(CurD, VD); - VD->demoteThisDefinitionToDeclaration(); - break; - } - } - } } static bool isUndeducedReturnType(QualType T) { diff --git a/clang/test/Modules/pr172241.cppm b/clang/test/Modules/pr172241.cppm new file mode 100644 index 0000000000000..3eb885e8b2d9f --- /dev/null +++ b/clang/test/Modules/pr172241.cppm @@ -0,0 +1,47 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/m.cppm -emit-module-interface -o %t/m.pcm +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/use.cpp -fmodule-file=m=%t/m.pcm -emit-llvm -o - | FileCheck %t/use.cpp +// +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/m.cppm -emit-reduced-module-interface -o %t/m.pcm +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple %t/use.cpp -fmodule-file=m=%t/m.pcm -emit-llvm -o - | FileCheck %t/use.cpp + +//--- header.h +#pragma once + +template <unsigned T> +class Templ { +public: + void lock() { __set_locked_bit(); } + +private: + static constexpr auto __set_locked_bit = [](){}; +}; + +class JT { +public: + ~JT() { + Templ<4> state; + state.lock(); + } +}; + +//--- m.cppm +module; +#include "header.h" +export module m; +export struct M { + JT jt; +}; +//--- use.cpp +#include "header.h" +import m; + +int main() { + M m; + return 0; +} + +// CHECK: @_ZN5TemplILj4EE16__set_locked_bitE = {{.*}}linkonce_odr _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
