https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/76119
>From 65fdddbd501b36f7dcef2db3fd25a7a4a1f80a0b Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <yedeng...@linux.alibaba.com> Date: Thu, 21 Dec 2023 11:24:14 +0800 Subject: [PATCH] [Modules] [HeaderSearch] Don't reenter headers if it is pragma once of #import Close https://github.com/llvm/llvm-project/issues/73023 The direct issue of https://github.com/llvm/llvm-project/issues/73023 is that we entered a header which is marked as pragma once since the compiler think it is OK if there is controlling macro. It doesn't make sense. I feel like it should be sufficient to skip it after we see the '#pragma once'. >From the context, it looks like the workaround is primarily for ObjectiveC. So we might need reviewers from OC. --- clang/lib/Lex/HeaderSearch.cpp | 78 +++++++++++++++++----------------- clang/test/Modules/pr73023.cpp | 19 +++++++++ 2 files changed, 58 insertions(+), 39 deletions(-) create mode 100644 clang/test/Modules/pr73023.cpp diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 0f1090187734ff2..dfa974e9a67ede3 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -1408,57 +1408,57 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP, // Get information about this file. HeaderFileInfo &FileInfo = getFileInfo(File); - // FIXME: this is a workaround for the lack of proper modules-aware support - // for #import / #pragma once - auto TryEnterImported = [&]() -> bool { - if (!ModulesEnabled) - return false; - // Ensure FileInfo bits are up to date. - ModMap.resolveHeaderDirectives(File); - // Modules with builtins are special; multiple modules use builtins as - // modular headers, example: - // - // module stddef { header "stddef.h" export * } - // - // After module map parsing, this expands to: - // - // module stddef { - // header "/path_to_builtin_dirs/stddef.h" - // textual "stddef.h" - // } - // - // It's common that libc++ and system modules will both define such - // submodules. Make sure cached results for a builtin header won't - // prevent other builtin modules from potentially entering the builtin - // header. Note that builtins are header guarded and the decision to - // actually enter them is postponed to the controlling macros logic below. - bool TryEnterHdr = false; - if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader) - TryEnterHdr = ModMap.isBuiltinHeader(File); - - // Textual headers can be #imported from different modules. Since ObjC - // headers find in the wild might rely only on #import and do not contain - // controlling macros, be conservative and only try to enter textual headers - // if such macro is present. - if (!FileInfo.isModuleHeader && - FileInfo.getControllingMacro(ExternalLookup)) - TryEnterHdr = true; - return TryEnterHdr; - }; - // If this is a #import directive, check that we have not already imported // this header. if (isImport) { // If this has already been imported, don't import it again. FileInfo.isImport = true; + // FIXME: this is a workaround for the lack of proper modules-aware support + // for #import / #pragma once + auto TryEnterImported = [&]() -> bool { + if (!ModulesEnabled) + return false; + // Ensure FileInfo bits are up to date. + ModMap.resolveHeaderDirectives(File); + // Modules with builtins are special; multiple modules use builtins as + // modular headers, example: + // + // module stddef { header "stddef.h" export * } + // + // After module map parsing, this expands to: + // + // module stddef { + // header "/path_to_builtin_dirs/stddef.h" + // textual "stddef.h" + // } + // + // It's common that libc++ and system modules will both define such + // submodules. Make sure cached results for a builtin header won't + // prevent other builtin modules from potentially entering the builtin + // header. Note that builtins are header guarded and the decision to + // actually enter them is postponed to the controlling macros logic below. + bool TryEnterHdr = false; + if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader) + TryEnterHdr = ModMap.isBuiltinHeader(File); + + // Textual headers can be #imported from different modules. Since ObjC + // headers find in the wild might rely only on #import and do not contain + // controlling macros, be conservative and only try to enter textual + // headers if such macro is present. + if (!FileInfo.isModuleHeader && + FileInfo.getControllingMacro(ExternalLookup)) + TryEnterHdr = true; + return TryEnterHdr; + }; + // Has this already been #import'ed or #include'd? if (PP.alreadyIncluded(File) && !TryEnterImported()) return false; } else { // Otherwise, if this is a #include of a file that was previously #import'd // or if this is the second #include of a #pragma once file, ignore it. - if ((FileInfo.isPragmaOnce || FileInfo.isImport) && !TryEnterImported()) + if (FileInfo.isPragmaOnce || FileInfo.isImport) return false; } diff --git a/clang/test/Modules/pr73023.cpp b/clang/test/Modules/pr73023.cpp new file mode 100644 index 000000000000000..19d7df6ce8a4ce9 --- /dev/null +++ b/clang/test/Modules/pr73023.cpp @@ -0,0 +1,19 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/test.cpp -fsyntax-only -verify + +//--- i.h +#ifndef I_H +#pragma once +struct S{}; +#endif + +//--- test.cpp +// expected-no-diagnostics +#include "i.h" + +int foo() { + return sizeof(S); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
