llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Aiden Grossman (boomanaiden154) <details> <summary>Changes</summary> Reverts llvm/llvm-project#<!-- -->177117 See #<!-- -->181076 for details of a miscompile caused by this. --- Full diff: https://github.com/llvm/llvm-project/pull/181384.diff 6 Files Affected: - (modified) clang/lib/Sema/SemaType.cpp (+24-52) - (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+14) - (removed) clang/test/Modules/demote-var-def.cpp (-94) - (removed) clang/test/Modules/pr149404-02.cppm (-104) - (removed) clang/test/Modules/pr172241.cppm (-47) - (removed) clang/test/Modules/var-inst-def.cppm (-110) ``````````diff diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index a6d4b989cae3d..c0c0ab7a09c72 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -9309,59 +9309,11 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, NamedDecl **Suggested, // If this definition was instantiated from a template, map back to the // pattern from which it was instantiated. - if (isa<TagDecl>(D) && cast<TagDecl>(D)->isBeingDefined()) + if (isa<TagDecl>(D) && cast<TagDecl>(D)->isBeingDefined()) { // We're in the middle of defining it; this definition should be treated // as visible. return true; - - auto DefinitionIsAcceptable = [&](NamedDecl *D) { - // The (primary) definition might be in a visible module. - if (isAcceptable(D, Kind)) - return true; - - // A visible module might have a merged definition instead. - if (D->isModulePrivate() ? hasMergedDefinitionInCurrentModule(D) - : hasVisibleMergedDefinition(D)) { - if (CodeSynthesisContexts.empty() && - !getLangOpts().ModulesLocalVisibility) { - // Cache the fact that this definition is implicitly visible because - // there is a visible merged definition. - D->setVisibleDespiteOwningModule(); - } - return true; - } - - return false; - }; - auto IsDefinition = [](NamedDecl *D) { - if (auto *RD = dyn_cast<CXXRecordDecl>(D)) - return RD->isThisDeclarationADefinition(); - if (auto *ED = dyn_cast<EnumDecl>(D)) - return ED->isThisDeclarationADefinition(); - if (auto *FD = dyn_cast<FunctionDecl>(D)) - return FD->isThisDeclarationADefinition(); - if (auto *VD = dyn_cast<VarDecl>(D)) - return VD->isThisDeclarationADefinition() == VarDecl::Definition; - llvm_unreachable("unexpected decl type"); - }; - auto FoundAcceptableDefinition = [&](NamedDecl *D) { - if (!isa<CXXRecordDecl, FunctionDecl, EnumDecl, VarDecl>(D)) - return DefinitionIsAcceptable(D); - - for (auto *RD : D->redecls()) { - auto *ND = cast<NamedDecl>(RD); - if (!IsDefinition(ND)) - continue; - if (DefinitionIsAcceptable(ND)) { - *Suggested = ND; - return true; - } - } - - return false; - }; - - if (auto *RD = dyn_cast<CXXRecordDecl>(D)) { + } else if (auto *RD = dyn_cast<CXXRecordDecl>(D)) { if (auto *Pattern = RD->getTemplateInstantiationPattern()) RD = Pattern; D = RD->getDefinition(); @@ -9400,14 +9352,34 @@ bool Sema::hasAcceptableDefinition(NamedDecl *D, NamedDecl **Suggested, *Suggested = D; - if (FoundAcceptableDefinition(D)) + auto DefinitionIsAcceptable = [&] { + // The (primary) definition might be in a visible module. + if (isAcceptable(D, Kind)) + return true; + + // A visible module might have a merged definition instead. + if (D->isModulePrivate() ? hasMergedDefinitionInCurrentModule(D) + : hasVisibleMergedDefinition(D)) { + if (CodeSynthesisContexts.empty() && + !getLangOpts().ModulesLocalVisibility) { + // Cache the fact that this definition is implicitly visible because + // there is a visible merged definition. + D->setVisibleDespiteOwningModule(); + } + return true; + } + + return false; + }; + + if (DefinitionIsAcceptable()) return true; // The external source may have additional definitions of this entity that are // visible, so complete the redeclaration chain now and ask again. if (auto *Source = Context.getExternalSource()) { Source->CompleteRedeclChain(D); - return FoundAcceptableDefinition(D); + return DefinitionIsAcceptable(); } return false; diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index f0fb247f1afb9..f8e9caa3f5d1d 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -3642,9 +3642,23 @@ template<> void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, Redeclarable<VarDecl> *D, Decl *Previous, Decl *Canon) { + auto *VD = static_cast<VarDecl *>(D); 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/demote-var-def.cpp b/clang/test/Modules/demote-var-def.cpp deleted file mode 100644 index 811440dd736f2..0000000000000 --- a/clang/test/Modules/demote-var-def.cpp +++ /dev/null @@ -1,94 +0,0 @@ -// RUN: rm -rf %t -// RUN: mkdir -p %t -// RUN: split-file %s %t -// RUN: cd %t -// -// DEFINE: %{common-flags}= -I %t -isystem %t -xc++ -std=c++20 -fmodules -// -// RUN: mkdir -p %t/b2 -// RUN: mkdir -p %t/b1 -// RUN: %clang_cc1 %{common-flags} -emit-module -fmodule-name=module_d \ -// RUN: d.cppmap -o d.pcm -// RUN: %clang_cc1 %{common-flags} -emit-module -fmodule-name=module_a \ -// RUN: -fmodule-file=d.pcm a.cppmap -o a.pcm -// RUN: %clang_cc1 %{common-flags} -emit-module -fmodule-name=module_b2 \ -// RUN: -fmodule-file=a.pcm b2/b.cppmap -o b2/b.pcm -// RUN: %clang_cc1 %{common-flags} -emit-module -fmodule-name=module_b1 \ -// RUN: -fmodule-file=b2/b.pcm b1/b.cppmap -o b1/b.pcm -// RUN: %clang_cc1 %{common-flags} -emit-module -fmodule-name=module_f \ -// RUN: -fmodule-file=b1/b.pcm f.cppmap -o f.pcm -// RUN: %clang_cc1 %{common-flags} -emit-module -fmodule-name=module_c \ -// RUN: -fmodule-file=f.pcm c.cppmap -o c.pcm -// RUN: %clang_cc1 %{common-flags} -emit-module \ -// RUN: -fmodule-name=module_e e.cppmap -o e.pcm -// -// RUN: %clang_cc1 %{common-flags} \ -// RUN: -fmodule-file=c.pcm -fmodule-file=e.pcm \ -// RUN: src.cpp -o src.pic.o - -//--- invoke.h -#ifndef _LIBCPP___TYPE_TRAITS_IS_SAME_H -#define _LIBCPP___TYPE_TRAITS_IS_SAME_H -namespace std { inline namespace _LIBCPP_ABI_NAMESPACE { -template <class _Tp, class _Up> -constexpr bool is_same_v = __is_same(_Tp, _Up); -} } -#endif - -//--- memory -#include <invoke.h> -namespace std { inline namespace _LIBCPP_ABI_NAMESPACE { -template <class _Tp> -using __decay_t = __decay(_Tp); -template <class _Tp> -using decay_t = __decay_t<_Tp>; -} } - -//--- other.h -#include <invoke.h> - -//--- a.cppmap -module "module_a" { -} - -//--- b1/b.cppmap -module "module_b1" { -} - -//--- b2/b.cppmap -module "module_b2" { -} - -//--- c.cppmap -module "module_c" { -} - -//--- d.cppmap -module "module_d" { - header "d.h" -} - -//--- d.h -#include <other.h> - -//--- e.cppmap -module "module_e" { - header "e.h" -} - -//--- e.h -#include <memory> - -//--- f.cppmap -module "module_f" { -} - -//--- src.cpp -#include <d.h> -#include <memory> -template <typename T> -concept coroutine_result = - std::is_same_v<std::decay_t<T>, T>; -template <coroutine_result R> -class Co; -using T = Co<void>; diff --git a/clang/test/Modules/pr149404-02.cppm b/clang/test/Modules/pr149404-02.cppm deleted file mode 100644 index 291619ea05b8a..0000000000000 --- a/clang/test/Modules/pr149404-02.cppm +++ /dev/null @@ -1,104 +0,0 @@ -// RUN: rm -rf %t -// RUN: mkdir -p %t -// RUN: split-file %s %t - -// RUN: %clang_cc1 -std=c++20 -emit-module-interface -o %t/format.pcm %t/format.cppm -// RUN: %clang_cc1 -std=c++20 -emit-module-interface -o %t/includes_in_gmf.pcm %t/includes_in_gmf.cppm -// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/test.cpp -verify -fsyntax-only - -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface -o %t/format.pcm %t/format.cppm -// RUN: %clang_cc1 -std=c++20 -emit-reduced-module-interface -o %t/includes_in_gmf.pcm %t/includes_in_gmf.cppm -// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/test.cpp -verify -fsyntax-only - -//--- format.h -#pragma once - -namespace test { - -template <class _Tp> -struct type_identity { - typedef _Tp type; -}; - -template <class _Tp> -using type_identity_t = typename type_identity<_Tp>::type; - - -template <class _Tp, class _CharT> -struct formatter -{ - formatter() = delete; -}; - -template <> -struct formatter<char, char> -{}; - -template <class _CharT, class... _Args> -struct basic_format_string { - static inline const int __handles_{ [] { - formatter<char, _CharT> f; - (void)f; - return 0; - }() }; - - consteval basic_format_string(const _CharT*) { - (void)__handles_; - } -}; - -template <class... _Args> -using wformat_string = basic_format_string<wchar_t, type_identity_t<_Args>...>; - -template <class... _Args> -using format_string = basic_format_string<char, type_identity_t<_Args>...>; - -template <class... _Args> -void format(format_string<_Args...> __fmt, _Args&&... __args) {} - -template <class... _Args> -void format(wformat_string<_Args...> __fmt, _Args&&... __args) {} - -} - -//--- format.cppm -module; -#include "format.h" -export module format; - -export namespace test { - using test::format; - using test::formatter; - using test::format_string; -} - -auto something() -> void -{ - auto a = 'a'; - test::format("{}", a); -} - -//--- includes_in_gmf.cppm -module; -#include "format.h" -export module includes_in_gmf; - -namespace test { - using test::format; - using test::formatter; - using test::format_string; -} - -//--- test.cpp -// expected-no-diagnostics -import format; -import includes_in_gmf; - -auto what() -> void -{ - auto a = 'a'; - test::format("{}", a); - - constexpr auto fs = "{}"; // test::format_string<char>{ "{}" }; // <- same result even passing exact param type - test::format(fs, 'r'); -} diff --git a/clang/test/Modules/pr172241.cppm b/clang/test/Modules/pr172241.cppm deleted file mode 100644 index 3eb885e8b2d9f..0000000000000 --- a/clang/test/Modules/pr172241.cppm +++ /dev/null @@ -1,47 +0,0 @@ -// 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 diff --git a/clang/test/Modules/var-inst-def.cppm b/clang/test/Modules/var-inst-def.cppm deleted file mode 100644 index 1414ec76c7be5..0000000000000 --- a/clang/test/Modules/var-inst-def.cppm +++ /dev/null @@ -1,110 +0,0 @@ -// RUN: rm -rf %t -// RUN: mkdir -p %t -// RUN: split-file %s %t -// RUN: cd %t -// -// RUN: %clang_cc1 -fmodule-name=A -xc++ -emit-module -fmodules \ -// RUN: -fno-cxx-modules -fno-implicit-modules \ -// RUN: -fmodule-map-file-home-is-cwd -std=c++20 -I. a.modulemap -o a.pcm -// -// RUN: %clang_cc1 -fmodule-name=B -xc++ -emit-module -fmodules \ -// RUN: -fno-cxx-modules -fno-implicit-modules \ -// RUN: -fmodule-map-file-home-is-cwd -std=c++20 -I. b.modulemap -o b.pcm -// -// RUN: %clang_cc1 -fmodule-name=C -xc++ -emit-module -fmodules \ -// RUN: -fno-cxx-modules -fno-implicit-modules \ -// RUN: -fmodule-map-file-home-is-cwd -std=c++20 -I. c.modulemap -o c.pcm -// -// RUN: %clang_cc1 -fno-cxx-modules -fmodules -fno-implicit-modules \ -// RUN: -fmodule-map-file-home-is-cwd \ -// RUN: -fmodule-file=a.pcm -fmodule-file=b.pcm -fmodule-file=c.pcm \ -// RUN: -std=c++20 -I. main.cpp -o /dev/null - -//--- a.modulemap -module "A" { header "a.h" } -//--- b.modulemap -module "B" { header "b.h" } -//--- c.modulemap -module "C" { header "c.h" } - -//--- common.h -#pragma once -#include "stl.h" - -//--- a.h -#pragma once -#include "common.h" -#include "repro.h" - -//--- b.h -#pragma once -#include "common.h" -#include "repro.h" - -//--- c.h -#pragma once -#include "common.h" -#include "repro.h" - -//--- repro.h -#pragma once -#include "stl.h" - -namespace k { -template <template <typename> class , typename > -struct is_instantiation : std::integral_constant<bool, false> {}; -template <template <typename> class C, typename T> -constexpr bool is_instantiation_v = is_instantiation<C, T>::value; -} - -struct ThreadState; - -namespace cc::subtle { -template <typename T> -class U; -} -namespace cc { -template <typename T> class Co; -namespace internal { -template <typename T> -class Promise { - static_assert(!k::is_instantiation_v<subtle::U, T>); -}; -} -} - -//--- stl.h -#pragma once -namespace std { -inline namespace abi { -template <class _Tp, _Tp __v> -struct integral_constant { - static const _Tp value = __v; -}; -template <class _Tp, class _Up> -constexpr bool is_same_v = __is_same(_Tp, _Up); -template <class _Tp> -using decay_t = __decay(_Tp); - -template <class> -struct __invoke_result_impl ; -template <class... _Args> -using invoke_result_t = __invoke_result_impl<_Args...>; -} -} - -//--- main.cpp -#include "stl.h" -#include "a.h" - -namespace cc { -template <typename F> - requires k::is_instantiation_v<Co, std::invoke_result_t<F>> -using result_type = - std::invoke_result_t<F>; -} -namespace cc::internal { -class final { - Promise<ThreadState> outgoing_work_; -}; -} `````````` </details> https://github.com/llvm/llvm-project/pull/181384 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
