[clang] [clang][modules] Avoid modules diagnostics for `__has_include()` (PR #71450)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/71450 After #70144 Clang started resolving module maps even for `__has_include()` expressions. The unintended consequence of this is that diagnostics related to module mis-use started trigerring. These diagnostics are clearly intended to catch actual usage of a header, so should not apply for `__has_include()` expressions that don't bring the contents of the header into the TU. >From f66c121e698142892b4c59163cae38cd10fc1e0c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 6 Nov 2023 13:05:23 -0800 Subject: [PATCH 1/2] Fix --- clang/lib/Lex/PPDirectives.cpp | 36 +++--- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index d97a103833c2fa6..956e2276f25b710 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -960,7 +960,6 @@ OptionalFileEntryRef Preprocessor::LookupFile( Module *RequestingModule = getModuleForLocation( FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes); - bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc); // If the header lookup mechanism may be relative to the current inclusion // stack, record the parent #includes. @@ -1041,13 +1040,8 @@ OptionalFileEntryRef Preprocessor::LookupFile( Filename, FilenameLoc, isAngled, FromDir, &CurDir, Includers, SearchPath, RelativePath, RequestingModule, SuggestedModule, IsMapped, IsFrameworkFound, SkipCache, BuildSystemModule, OpenFile, CacheFailures); - if (FE) { -if (SuggestedModule && !LangOpts.AsmPreprocessor) - HeaderInfo.getModuleMap().diagnoseHeaderInclusion( - RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc, - Filename, *FE); + if (FE) return FE; - } OptionalFileEntryRef CurFileEnt; // Otherwise, see if this is a subframework header. If so, this is relative @@ -1058,10 +1052,6 @@ OptionalFileEntryRef Preprocessor::LookupFile( if (OptionalFileEntryRef FE = HeaderInfo.LookupSubframeworkHeader( Filename, *CurFileEnt, SearchPath, RelativePath, RequestingModule, SuggestedModule)) { -if (SuggestedModule && !LangOpts.AsmPreprocessor) - HeaderInfo.getModuleMap().diagnoseHeaderInclusion( - RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc, - Filename, *FE); return FE; } } @@ -1073,10 +1063,6 @@ OptionalFileEntryRef Preprocessor::LookupFile( if (OptionalFileEntryRef FE = HeaderInfo.LookupSubframeworkHeader( Filename, *CurFileEnt, SearchPath, RelativePath, RequestingModule, SuggestedModule)) { - if (SuggestedModule && !LangOpts.AsmPreprocessor) -HeaderInfo.getModuleMap().diagnoseHeaderInclusion( -RequestingModule, RequestingModuleIsModuleInterface, -FilenameLoc, Filename, *FE); return FE; } } @@ -2027,12 +2013,28 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport( const FileEntry *LookupFromFile, StringRef &LookupFilename, SmallVectorImpl &RelativePath, SmallVectorImpl &SearchPath, ModuleMap::KnownHeader &SuggestedModule, bool isAngled) { + auto DiagnoseHeaderInclusion = [&](FileEntryRef FE) { +if (LangOpts.AsmPreprocessor) + return; + +Module *RequestingModule = getModuleForLocation( +FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes); +bool RequestingModuleIsModuleInterface = +!SourceMgr.isInMainFile(FilenameLoc); + +HeaderInfo.getModuleMap().diagnoseHeaderInclusion( +RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc, +Filename, FE); + }; + OptionalFileEntryRef File = LookupFile( FilenameLoc, LookupFilename, isAngled, LookupFrom, LookupFromFile, CurDir, Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped, &IsFrameworkFound); - if (File) + if (File) { +DiagnoseHeaderInclusion(*File); return File; + } // Give the clients a chance to silently skip this include. if (Callbacks && Callbacks->FileNotFound(Filename)) @@ -2051,6 +2053,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport( &SuggestedModule, &IsMapped, /*IsFrameworkFound=*/nullptr); if (File) { + DiagnoseHeaderInclusion(*File); Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal) << Filename << IsImportDecl << FixItHint::CreateReplacement(FilenameRange, @@ -2081,6 +2084,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport( Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped, /*IsFrameworkFound=*/nullptr); if (File) { + DiagnoseHeaderInclusion(*File)
[clang] [clang][modules] Avoid modules diagnostics for `__has_include()` (PR #71450)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/71450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 4f31d32 - [clang] Improve `SourceManager::PrintStats()`
Author: Jan Svoboda Date: 2023-11-06T14:45:04-08:00 New Revision: 4f31d328aa165e559c9f374dc3201657921f150d URL: https://github.com/llvm/llvm-project/commit/4f31d328aa165e559c9f374dc3201657921f150d DIFF: https://github.com/llvm/llvm-project/commit/4f31d328aa165e559c9f374dc3201657921f150d.diff LOG: [clang] Improve `SourceManager::PrintStats()` This fixes a typo ("SLocEntry's" -> "SLocEntries"), fixes capitalization ("Sloc" -> "SLoc") and adds extra information (capacity in bytes of `LoadedSLocEntryTable`). Added: Modified: clang/lib/Basic/SourceManager.cpp Removed: diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp index d627c233a3aa094..37734d3b10e78f7 100644 --- a/clang/lib/Basic/SourceManager.cpp +++ b/clang/lib/Basic/SourceManager.cpp @@ -2121,14 +2121,16 @@ void SourceManager::PrintStats() const { llvm::errs() << "\n*** Source Manager Stats:\n"; llvm::errs() << FileInfos.size() << " files mapped, " << MemBufferInfos.size() << " mem buffers mapped.\n"; - llvm::errs() << LocalSLocEntryTable.size() << " local SLocEntry's allocated (" + llvm::errs() << LocalSLocEntryTable.size() << " local SLocEntries allocated (" << llvm::capacity_in_bytes(LocalSLocEntryTable) - << " bytes of capacity), " - << NextLocalOffset << "B of Sloc address space used.\n"; + << " bytes of capacity), " << NextLocalOffset + << "B of SLoc address space used.\n"; llvm::errs() << LoadedSLocEntryTable.size() - << " loaded SLocEntries allocated, " + << " loaded SLocEntries allocated (" + << llvm::capacity_in_bytes(LoadedSLocEntryTable) + << " bytes of capacity), " << MaxLoadedOffset - CurrentLoadedOffset - << "B of Sloc address space used.\n"; + << "B of SLoc address space used.\n"; unsigned NumLineNumsComputed = 0; unsigned NumFileBytesMapped = 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8c09916 - [clang] Fix test after 4f31d32
Author: Jan Svoboda Date: 2023-11-06T14:56:25-08:00 New Revision: 8c099168509da532c26ba843eed612dc53b27afc URL: https://github.com/llvm/llvm-project/commit/8c099168509da532c26ba843eed612dc53b27afc DIFF: https://github.com/llvm/llvm-project/commit/8c099168509da532c26ba843eed612dc53b27afc.diff LOG: [clang] Fix test after 4f31d32 Added: Modified: clang/test/Lexer/update_consecutive_macro_address_space.c Removed: diff --git a/clang/test/Lexer/update_consecutive_macro_address_space.c b/clang/test/Lexer/update_consecutive_macro_address_space.c index cf3c406112b25bc..80ef4557591c39e 100644 --- a/clang/test/Lexer/update_consecutive_macro_address_space.c +++ b/clang/test/Lexer/update_consecutive_macro_address_space.c @@ -1,5 +1,5 @@ // RUN: %clang -cc1 -print-stats %s 2>&1 | FileCheck %s -// CHECK: 6 local SLocEntry's allocated +// CHECK: 6 local SLocEntries allocated // // Verify that the macro arg expansion is split to two file ids, we have 6 file // ids rather than 5: ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Avoid modules diagnostics for `__has_include()` (PR #71450)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/71450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)
@@ -52,6 +52,28 @@ static void optimizeHeaderSearchOpts(HeaderSearchOptions &Opts, Opts.UserEntries.push_back(Entries[Idx]); } +static void optimizeDiagnosticOpts(DiagnosticOptions &Opts, + bool IsSystemModule) { + // If this is not a system module or -Wsystem-headers was passed, don't + // optimize. + if (!IsSystemModule) +return; + bool Wsystem_headers = false; + for (StringRef Opt : Opts.Warnings) { +bool isPositive = !Opt.consume_front("no-"); +if (Opt == "system-headers") + Wsystem_headers = isPositive; + } + if (Wsystem_headers) +return; jansvoboda11 wrote: Could we reuse `clang::ProcessWarningOptions()` to avoid duplicating the logic? https://github.com/llvm/llvm-project/pull/71612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)
@@ -0,0 +1,84 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > %t/build/compile-commands.json +// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \ +// RUN: -j 1 -format experimental-full -optimize-args=system-warnings > %t/deps.db +// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t + +// CHECK: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": +// CHECK-NEXT: "command-line": [ +// CHECK-NOT: "-W +// CHECK:], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK:], +// CHECK-NEXT: "name": "A" +// CHECK-NEXT: }, +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": +// CHECK-NEXT: "command-line": [ +// CHECK-NOT: "-W +// CHECK:], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK:], +// CHECK-NEXT: "name": "B" +// CHECK-NEXT: } +// CHECK-NEXT: ], +// CHECK-NEXT: "translation-units": [ +// CHECK:] +// CHECK: } + +//--- build/compile-commands.json.in + +[ +{ + "directory": "DIR", + "command": "clang -c DIR/A.m -isystem modules/A -I modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps", + "file": "DIR/A.m" +}, +{ + "directory": "DIR", + "command": "clang -c DIR/B.m -isystem modules/A -I modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps -Wmaybe-unused", + "file": "DIR/B.m" +} jansvoboda11 wrote: I think we should probably also test the case where we don't optimize because of "-Wsystem-headers". https://github.com/llvm/llvm-project/pull/71612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)
@@ -0,0 +1,84 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/build/compile-commands.json.in > %t/build/compile-commands.json +// RUN: clang-scan-deps -compilation-database %t/build/compile-commands.json \ +// RUN: -j 1 -format experimental-full -optimize-args=system-warnings > %t/deps.db +// RUN: cat %t/deps.db | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t + +// CHECK: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": +// CHECK-NEXT: "command-line": [ +// CHECK-NOT: "-W +// CHECK:], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK:], +// CHECK-NEXT: "name": "A" +// CHECK-NEXT: }, +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": +// CHECK-NEXT: "command-line": [ +// CHECK-NOT: "-W +// CHECK:], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK:], +// CHECK-NEXT: "name": "B" +// CHECK-NEXT: } +// CHECK-NEXT: ], +// CHECK-NEXT: "translation-units": [ +// CHECK:] +// CHECK: } + +//--- build/compile-commands.json.in + +[ +{ + "directory": "DIR", + "command": "clang -c DIR/A.m -isystem modules/A -I modules/B -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-module-maps", jansvoboda11 wrote: What does this test? https://github.com/llvm/llvm-project/pull/71612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)
@@ -0,0 +1,84 @@ +// RUN: rm -rf %t jansvoboda11 wrote: Could you add a short summary of what this test is for? https://github.com/llvm/llvm-project/pull/71612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record (PR #70874)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/70874 >From 450c5adeb3ddc5bf7e01fcbe379c37b08d4de9d1 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 31 Oct 2023 16:35:38 -0700 Subject: [PATCH] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record --- clang/include/clang/Lex/HeaderSearchOptions.h | 8 +++- clang/lib/Serialization/ASTWriter.cpp | 4 ++-- .../DependencyScanning/DependencyScanningWorker.cpp | 2 ++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index 114af14dec7f5a8..fa2d0b502d72c19 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -247,6 +247,11 @@ class HeaderSearchOptions { LLVM_PREFERRED_TYPE(bool) unsigned ModulesSkipHeaderSearchPaths : 1; + /// Whether to entirely skip writing pragma diagnostic mappings. + /// Primarily used to speed up deserialization during dependency scanning. + LLVM_PREFERRED_TYPE(bool) + unsigned ModulesSkipPragmaDiagnosticMappings : 1; + LLVM_PREFERRED_TYPE(bool) unsigned ModulesHashContent : 1; @@ -270,7 +275,8 @@ class HeaderSearchOptions { ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false), ModulesValidateDiagnosticOptions(true), ModulesSkipDiagnosticOptions(false), -ModulesSkipHeaderSearchPaths(false), ModulesHashContent(false), +ModulesSkipHeaderSearchPaths(false), +ModulesSkipPragmaDiagnosticMappings(false), ModulesHashContent(false), ModulesStrictContextHash(false) {} /// AddPath - Add the \p Path path to the specified \p Group list. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 1e86566d81fbc02..0161ad10f3f2381 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1261,8 +1261,8 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, Stream.EmitRecord(HEADER_SEARCH_PATHS, Record); } - // Write out the diagnostic/pragma mappings. - WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule); + if (!HSOpts.ModulesSkipPragmaDiagnosticMappings) +WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule); // Header search entry usage. auto HSEntryUsage = PP.getHeaderSearchInfo().computeUserEntryUsage(); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index d1d3cc50cb25b83..c54e6d523800b6b 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -255,6 +255,8 @@ class DependencyScanningAction : public tooling::ToolAction { ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true; ScanInstance.getHeaderSearchOpts().ModulesSkipDiagnosticOptions = true; ScanInstance.getHeaderSearchOpts().ModulesSkipHeaderSearchPaths = true; +ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = +true; // Avoid some checks and module map parsing when loading PCM files. ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record (PR #70874)
https://github.com/jansvoboda11 ready_for_review https://github.com/llvm/llvm-project/pull/70874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record (PR #70874)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/70874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][DependencyScanner] Remove all warning flags when suppressing warnings (PR #71612)
https://github.com/jansvoboda11 approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/71612 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record (PR #70874)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/70874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/68157 >From e3a96bef47d029e1109dcad51840bab672c7351c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 3 Oct 2023 13:40:07 -0700 Subject: [PATCH 1/2] [clang] NFC: Deprecate `FileEntry::getName()` --- clang/include/clang/Basic/FileEntry.h | 1 + clang/unittests/Basic/FileManagerTest.cpp | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h index bc65463735488..6351aeae92e2c 100644 --- a/clang/include/clang/Basic/FileEntry.h +++ b/clang/include/clang/Basic/FileEntry.h @@ -394,6 +394,7 @@ class FileEntry { public: ~FileEntry(); + LLVM_DEPRECATED("Use FileEntryRef::getName() instead.", "") StringRef getName() const { return LastRef->getName(); } StringRef tryGetRealPathName() const { return RealPathName; } diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index bf30fabb7cd88..d32036d975ce9 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -284,7 +284,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F1Alias); ASSERT_FALSE(!F1Alias2); EXPECT_EQ("dir/f1.cpp", F1->getName()); - EXPECT_EQ("dir/f1.cpp", F1->getFileEntry().getName()); EXPECT_EQ("dir/f1.cpp", F1Alias->getName()); EXPECT_EQ("dir/f1.cpp", F1Alias2->getName()); EXPECT_EQ(&F1->getFileEntry(), &F1Alias->getFileEntry()); @@ -303,7 +302,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F2Alias); ASSERT_FALSE(!F2Alias2); EXPECT_EQ("dir/f2.cpp", F2->getName()); - EXPECT_EQ("dir/f2.cpp", F2->getFileEntry().getName()); EXPECT_EQ("dir/f2.cpp", F2Alias->getName()); EXPECT_EQ("dir/f2.cpp", F2Alias2->getName()); EXPECT_EQ(&F2->getFileEntry(), &F2Alias->getFileEntry()); >From d5f953f9f8880dd32ae21aa8cf3d0b30479b271c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 4 Dec 2023 11:55:07 -0800 Subject: [PATCH 2/2] [clang] Keep testing `getName()`, disable deprecation warnings --- clang/unittests/Basic/FileManagerTest.cpp | 6 ++ llvm/include/llvm/Support/Compiler.h | 19 +++ 2 files changed, 25 insertions(+) diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index d32036d975ce9..9de8b72bf5a78 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -284,6 +284,9 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F1Alias); ASSERT_FALSE(!F1Alias2); EXPECT_EQ("dir/f1.cpp", F1->getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN + EXPECT_EQ("dir/f1.cpp", F1->getFileEntry().getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END EXPECT_EQ("dir/f1.cpp", F1Alias->getName()); EXPECT_EQ("dir/f1.cpp", F1Alias2->getName()); EXPECT_EQ(&F1->getFileEntry(), &F1Alias->getFileEntry()); @@ -302,6 +305,9 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F2Alias); ASSERT_FALSE(!F2Alias2); EXPECT_EQ("dir/f2.cpp", F2->getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN + EXPECT_EQ("dir/f2.cpp", F2->getFileEntry().getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END EXPECT_EQ("dir/f2.cpp", F2Alias->getName()); EXPECT_EQ("dir/f2.cpp", F2Alias2->getName()); EXPECT_EQ(&F2->getFileEntry(), &F2Alias->getFileEntry()); diff --git a/llvm/include/llvm/Support/Compiler.h b/llvm/include/llvm/Support/Compiler.h index 6b13952bb2f41..26a46a279e7b2 100644 --- a/llvm/include/llvm/Support/Compiler.h +++ b/llvm/include/llvm/Support/Compiler.h @@ -157,6 +157,25 @@ #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]] #endif +// clang-format off +#if defined(__clang__) || defined(__GNUC__) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"") +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END \ + _Pragma("GCC diagnostic pop") +#elif defined(_MSC_VER) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ + _Pragma("warning(push)") \ + _Pragma("warning(disable : 4996)") +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END \ + _Pragma("warning(pop)") +#else +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END +#endif +// clang-format on + // Indicate that a non-static, non-const C++ member function reinitializes // the entire object to a known state, independent of the previous state of // the object.
[clang] [clang][modules] Reset codegen options. (PR #74006)
https://github.com/jansvoboda11 approved this pull request. LGTM, nice! https://github.com/llvm/llvm-project/pull/74006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
@@ -284,7 +284,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F1Alias); ASSERT_FALSE(!F1Alias2); EXPECT_EQ("dir/f1.cpp", F1->getName()); - EXPECT_EQ("dir/f1.cpp", F1->getFileEntry().getName()); jansvoboda11 wrote: I created `LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_{BEGIN,END}`. Let me know if you think this makes sense to keep in LLVM's Support library, or would make more local. https://github.com/llvm/llvm-project/pull/68157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/67839 >From 9c798ed914b0008d98587c94f8ee3bb914412215 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Fri, 8 Sep 2023 16:39:10 -0700 Subject: [PATCH 1/2] [clang][tidy] Ensure rewriter has the correct CWD This reverts commit 65331da0032ab4253a4bc0ddcb2da67664bd86a9. --- clang-tools-extra/clang-tidy/ClangTidy.cpp | 8 clang/lib/Rewrite/Rewriter.cpp | 13 +++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 4b1a67b6dd98a..8805864a8537a 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -227,6 +227,14 @@ class ErrorReporter { llvm::errs() << "Can't apply replacements for file " << File << "\n"; } } + + auto BuildDir = Context.getCurrentBuildDirectory(); + if (!BuildDir.empty()) +Rewrite.getSourceMgr() +.getFileManager() +.getVirtualFileSystem() +.setCurrentWorkingDirectory(BuildDir); + if (Rewrite.overwriteChangedFiles()) { llvm::errs() << "clang-tidy failed to apply suggested fixes.\n"; } else { diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp index ef2858990dd95..0896221dd0bde 100644 --- a/clang/lib/Rewrite/Rewriter.cpp +++ b/clang/lib/Rewrite/Rewriter.cpp @@ -412,12 +412,13 @@ bool Rewriter::overwriteChangedFiles() { unsigned OverwriteFailure = Diag.getCustomDiagID( DiagnosticsEngine::Error, "unable to overwrite file %0: %1"); for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) { -const FileEntry *Entry = getSourceMgr().getFileEntryForID(I->first); -if (auto Error = -llvm::writeToOutput(Entry->getName(), [&](llvm::raw_ostream &OS) { - I->second.write(OS); - return llvm::Error::success(); -})) { +OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first); +llvm::SmallString<128> Path(Entry->getName()); +getSourceMgr().getFileManager().makeAbsolutePath(Path); +if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream &OS) { + I->second.write(OS); + return llvm::Error::success(); +})) { Diag.Report(OverwriteFailure) << Entry->getName() << llvm::toString(std::move(Error)); AllWritten = false; >From 225e4f20385b9b8890639002f7afd8bb476d738b Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 4 Dec 2023 14:48:58 -0800 Subject: [PATCH 2/2] [clang][tidy] Keep track of CWD, improve test coverage --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 30 +++ .../Inputs/compilation-database/template.json | 9 -- .../clang-tidy-run-with-database.cpp | 11 --- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 8805864a8537a..36ea5a39875e5 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -147,7 +147,8 @@ class ErrorReporter { Files.makeAbsolutePath(FixAbsoluteFilePath); tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(), Repl.getLength(), Repl.getReplacementText()); -Replacements &Replacements = FileReplacements[R.getFilePath()]; +auto &Entry = FileReplacements[R.getFilePath()]; +Replacements &Replacements = Entry.Replacements; llvm::Error Err = Replacements.add(R); if (Err) { // FIXME: Implement better conflict handling. @@ -174,6 +175,7 @@ class ErrorReporter { } FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset()); FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied)); +Entry.BuildDir = Error.BuildDirectory; } } } @@ -189,9 +191,12 @@ class ErrorReporter { void finish() { if (TotalFixes > 0) { - Rewriter Rewrite(SourceMgr, LangOpts); + bool AnyNotWritten = false; for (const auto &FileAndReplacements : FileReplacements) { +Rewriter Rewrite(SourceMgr, LangOpts); StringRef File = FileAndReplacements.first(); +Files.getVirtualFileSystem().setCurrentWorkingDirectory( +FileAndReplacements.second.BuildDir); llvm::ErrorOr> Buffer = SourceMgr.getFileManager().getBufferForFile(File); if (!Buffer) { @@ -208,8 +213,8 @@ class ErrorReporter { continue; } llvm::Expected Replacements = -format::cleanupAroundReplacements(Code, FileAndReplacements.second, - *Style); +format::cleanupAroundReplacements( +C
[clang] [clang-tools-extra] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
@@ -227,6 +227,14 @@ class ErrorReporter { llvm::errs() << "Can't apply replacements for file " << File << "\n"; } } + + auto BuildDir = Context.getCurrentBuildDirectory(); jansvoboda11 wrote: You're right, the `clang-tidy/infrastructure/clang-tidy-run-with-database.cpp` test kept passing just by chance. I added extra test case to prove that we need to keep closer track for each file what was the CWD when we processed it. https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/67839 >From 9c798ed914b0008d98587c94f8ee3bb914412215 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Fri, 8 Sep 2023 16:39:10 -0700 Subject: [PATCH 1/3] [clang][tidy] Ensure rewriter has the correct CWD This reverts commit 65331da0032ab4253a4bc0ddcb2da67664bd86a9. --- clang-tools-extra/clang-tidy/ClangTidy.cpp | 8 clang/lib/Rewrite/Rewriter.cpp | 13 +++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 4b1a67b6dd98a..8805864a8537a 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -227,6 +227,14 @@ class ErrorReporter { llvm::errs() << "Can't apply replacements for file " << File << "\n"; } } + + auto BuildDir = Context.getCurrentBuildDirectory(); + if (!BuildDir.empty()) +Rewrite.getSourceMgr() +.getFileManager() +.getVirtualFileSystem() +.setCurrentWorkingDirectory(BuildDir); + if (Rewrite.overwriteChangedFiles()) { llvm::errs() << "clang-tidy failed to apply suggested fixes.\n"; } else { diff --git a/clang/lib/Rewrite/Rewriter.cpp b/clang/lib/Rewrite/Rewriter.cpp index ef2858990dd95..0896221dd0bde 100644 --- a/clang/lib/Rewrite/Rewriter.cpp +++ b/clang/lib/Rewrite/Rewriter.cpp @@ -412,12 +412,13 @@ bool Rewriter::overwriteChangedFiles() { unsigned OverwriteFailure = Diag.getCustomDiagID( DiagnosticsEngine::Error, "unable to overwrite file %0: %1"); for (buffer_iterator I = buffer_begin(), E = buffer_end(); I != E; ++I) { -const FileEntry *Entry = getSourceMgr().getFileEntryForID(I->first); -if (auto Error = -llvm::writeToOutput(Entry->getName(), [&](llvm::raw_ostream &OS) { - I->second.write(OS); - return llvm::Error::success(); -})) { +OptionalFileEntryRef Entry = getSourceMgr().getFileEntryRefForID(I->first); +llvm::SmallString<128> Path(Entry->getName()); +getSourceMgr().getFileManager().makeAbsolutePath(Path); +if (auto Error = llvm::writeToOutput(Path, [&](llvm::raw_ostream &OS) { + I->second.write(OS); + return llvm::Error::success(); +})) { Diag.Report(OverwriteFailure) << Entry->getName() << llvm::toString(std::move(Error)); AllWritten = false; >From 225e4f20385b9b8890639002f7afd8bb476d738b Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 4 Dec 2023 14:48:58 -0800 Subject: [PATCH 2/3] [clang][tidy] Keep track of CWD, improve test coverage --- clang-tools-extra/clang-tidy/ClangTidy.cpp| 30 +++ .../Inputs/compilation-database/template.json | 9 -- .../clang-tidy-run-with-database.cpp | 11 --- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 8805864a8537a..36ea5a39875e5 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -147,7 +147,8 @@ class ErrorReporter { Files.makeAbsolutePath(FixAbsoluteFilePath); tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(), Repl.getLength(), Repl.getReplacementText()); -Replacements &Replacements = FileReplacements[R.getFilePath()]; +auto &Entry = FileReplacements[R.getFilePath()]; +Replacements &Replacements = Entry.Replacements; llvm::Error Err = Replacements.add(R); if (Err) { // FIXME: Implement better conflict handling. @@ -174,6 +175,7 @@ class ErrorReporter { } FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset()); FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied)); +Entry.BuildDir = Error.BuildDirectory; } } } @@ -189,9 +191,12 @@ class ErrorReporter { void finish() { if (TotalFixes > 0) { - Rewriter Rewrite(SourceMgr, LangOpts); + bool AnyNotWritten = false; for (const auto &FileAndReplacements : FileReplacements) { +Rewriter Rewrite(SourceMgr, LangOpts); StringRef File = FileAndReplacements.first(); +Files.getVirtualFileSystem().setCurrentWorkingDirectory( +FileAndReplacements.second.BuildDir); llvm::ErrorOr> Buffer = SourceMgr.getFileManager().getBufferForFile(File); if (!Buffer) { @@ -208,8 +213,8 @@ class ErrorReporter { continue; } llvm::Expected Replacements = -format::cleanupAroundReplacements(Code, FileAndReplacements.second, - *Style); +format::cleanupAroundReplacements( +C
[llvm] [clang] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
@@ -157,6 +157,25 @@ #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]] #endif +// clang-format off +#if defined(__clang__) || defined(__GNUC__) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ jansvoboda11 wrote: Maybe we could have a more generic macro? ```c++ #define DO_PRAGMA(x) _Pragma(#x) #if defined(__clang__) || defined(__GNUC__) #define LLVM_DISABLE_WARNING_BEGIN(GCC_FLAG, MSVC_NUMBER) \ DO_PRAGMA(GCC diagnostic push) \ DO_PRAGMA(GCC diagnostic ignored "-W" GCC_FLAG) #define LLVM_DISABLE_WARNING_END \ DO_PRAGMA(GCC diagnostic pop) #elif defined(_MSC_VER) #define LLVM_DISABLE_WARNING_BEGIN(GCC_FLAG, MSVC_NUMBER) \ DO_PRAGMA(warning(push))\ DO_PRAGMA(warning(disable: MSVC_NUMBER)) #define LLVM_DISABLE_WARNING_END \ DO_PRAGMA(warning(pop)) #else #define LLVM_DISABLE_WARNING_BEGIN(GCC_FLAG, MSVC_NUMBER) #define LLVM_DISABLE_WARNING_END #endif LLVM_DISABLE_WARNING_BEGIN("deprecated-declarations", 4996) EXPECT_EQ("dir/f2.cpp", F2->getFileEntry().getName()); LLVM_DISABLE_WARNING_END ``` https://github.com/llvm/llvm-project/pull/68157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] bb0f162 - [clang][tidy] Fix build failure after 07157db
Author: Jan Svoboda Date: 2023-12-05T15:52:57-08:00 New Revision: bb0f162b3acfab3146807ab1e01946596d9921f9 URL: https://github.com/llvm/llvm-project/commit/bb0f162b3acfab3146807ab1e01946596d9921f9 DIFF: https://github.com/llvm/llvm-project/commit/bb0f162b3acfab3146807ab1e01946596d9921f9.diff LOG: [clang][tidy] Fix build failure after 07157db Added: Modified: clang-tools-extra/clang-tidy/ClangTidy.cpp Removed: diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index baee1d205813b..565f044778c94 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -148,7 +148,7 @@ class ErrorReporter { tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(), Repl.getLength(), Repl.getReplacementText()); auto &Entry = FileReplacements[R.getFilePath()]; -Replacements &Replacements = Entry.Replacements; +Replacements &Replacements = Entry.Replaces; llvm::Error Err = Replacements.add(R); if (Err) { // FIXME: Implement better conflict handling. @@ -216,7 +216,7 @@ class ErrorReporter { } llvm::Expected Replacements = format::cleanupAroundReplacements( -Code, FileAndReplacements.second.Replacements, *Style); +Code, FileAndReplacements.second.Replaces, *Style); if (!Replacements) { llvm::errs() << llvm::toString(Replacements.takeError()) << "\n"; continue; @@ -303,7 +303,7 @@ class ErrorReporter { struct ReplacementsWithBuildDir { StringRef BuildDir; -Replacements Replacements; +Replacements Replaces; }; FileManager Files; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/68157 >From d015b32b5a48c84a94b6a34138f809caf94ed456 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 3 Oct 2023 13:40:07 -0700 Subject: [PATCH 1/3] [clang] NFC: Deprecate `FileEntry::getName()` --- clang/include/clang/Basic/FileEntry.h | 1 + clang/unittests/Basic/FileManagerTest.cpp | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h index bc65463735488..6351aeae92e2c 100644 --- a/clang/include/clang/Basic/FileEntry.h +++ b/clang/include/clang/Basic/FileEntry.h @@ -394,6 +394,7 @@ class FileEntry { public: ~FileEntry(); + LLVM_DEPRECATED("Use FileEntryRef::getName() instead.", "") StringRef getName() const { return LastRef->getName(); } StringRef tryGetRealPathName() const { return RealPathName; } diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index bf30fabb7cd88..d32036d975ce9 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -284,7 +284,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F1Alias); ASSERT_FALSE(!F1Alias2); EXPECT_EQ("dir/f1.cpp", F1->getName()); - EXPECT_EQ("dir/f1.cpp", F1->getFileEntry().getName()); EXPECT_EQ("dir/f1.cpp", F1Alias->getName()); EXPECT_EQ("dir/f1.cpp", F1Alias2->getName()); EXPECT_EQ(&F1->getFileEntry(), &F1Alias->getFileEntry()); @@ -303,7 +302,6 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F2Alias); ASSERT_FALSE(!F2Alias2); EXPECT_EQ("dir/f2.cpp", F2->getName()); - EXPECT_EQ("dir/f2.cpp", F2->getFileEntry().getName()); EXPECT_EQ("dir/f2.cpp", F2Alias->getName()); EXPECT_EQ("dir/f2.cpp", F2Alias2->getName()); EXPECT_EQ(&F2->getFileEntry(), &F2Alias->getFileEntry()); >From 42371d4ae0f4f4309da637a3b45c19e4b81f0b03 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 4 Dec 2023 11:55:07 -0800 Subject: [PATCH 2/3] [clang] Keep testing `getName()`, disable deprecation warnings --- clang/unittests/Basic/FileManagerTest.cpp | 6 ++ llvm/include/llvm/Support/Compiler.h | 19 +++ 2 files changed, 25 insertions(+) diff --git a/clang/unittests/Basic/FileManagerTest.cpp b/clang/unittests/Basic/FileManagerTest.cpp index d32036d975ce9..9de8b72bf5a78 100644 --- a/clang/unittests/Basic/FileManagerTest.cpp +++ b/clang/unittests/Basic/FileManagerTest.cpp @@ -284,6 +284,9 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F1Alias); ASSERT_FALSE(!F1Alias2); EXPECT_EQ("dir/f1.cpp", F1->getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN + EXPECT_EQ("dir/f1.cpp", F1->getFileEntry().getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END EXPECT_EQ("dir/f1.cpp", F1Alias->getName()); EXPECT_EQ("dir/f1.cpp", F1Alias2->getName()); EXPECT_EQ(&F1->getFileEntry(), &F1Alias->getFileEntry()); @@ -302,6 +305,9 @@ TEST_F(FileManagerTest, getFileRefReturnsCorrectNameForDifferentStatPath) { ASSERT_FALSE(!F2Alias); ASSERT_FALSE(!F2Alias2); EXPECT_EQ("dir/f2.cpp", F2->getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN + EXPECT_EQ("dir/f2.cpp", F2->getFileEntry().getName()); + LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END EXPECT_EQ("dir/f2.cpp", F2Alias->getName()); EXPECT_EQ("dir/f2.cpp", F2Alias2->getName()); EXPECT_EQ(&F2->getFileEntry(), &F2Alias->getFileEntry()); diff --git a/llvm/include/llvm/Support/Compiler.h b/llvm/include/llvm/Support/Compiler.h index 6b13952bb2f41..26a46a279e7b2 100644 --- a/llvm/include/llvm/Support/Compiler.h +++ b/llvm/include/llvm/Support/Compiler.h @@ -157,6 +157,25 @@ #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]] #endif +// clang-format off +#if defined(__clang__) || defined(__GNUC__) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"") +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END \ + _Pragma("GCC diagnostic pop") +#elif defined(_MSC_VER) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ + _Pragma("warning(push)") \ + _Pragma("warning(disable : 4996)") +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END \ + _Pragma("warning(pop)") +#else +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_END +#endif +// clang-format on + // Indicate that a non-static, non-const C++ member function reinitializes // the entire object to a known state, independent of the previous state of // the object. >From b49bef
[llvm] [clang] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
@@ -157,6 +157,25 @@ #define LLVM_DEPRECATED(MSG, FIX) [[deprecated(MSG)]] #endif +// clang-format off +#if defined(__clang__) || defined(__GNUC__) +#define LLVM_IGNORE_DEPRECATIONS_OF_DECLARATIONS_BEGIN \ jansvoboda11 wrote: OK, I used the suggested naming, but added `DECLARATIONS` to distinguish from semantics of the libc++ version that also ignores `-Wdeprecated`, which I don't want to (and I'm not sure there's an MSVC counterpart). I left this in `Compiler.h`, since this might be useful in other parts of LLVM and for downstream projects that have extra usages of `FileEntry::getName()`. https://github.com/llvm/llvm-project/pull/68157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang] NFC: Deprecate `FileEntry::getName()` (PR #68157)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/68157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] f1c08ee - [clang] NFCI: Use `FileEntryRef::getName()` in clangIndexSerialization
Author: Jan Svoboda Date: 2023-12-06T11:34:46-08:00 New Revision: f1c08eebb3bb5eb6e984bcca511381a9748c6705 URL: https://github.com/llvm/llvm-project/commit/f1c08eebb3bb5eb6e984bcca511381a9748c6705 DIFF: https://github.com/llvm/llvm-project/commit/f1c08eebb3bb5eb6e984bcca511381a9748c6705.diff LOG: [clang] NFCI: Use `FileEntryRef::getName()` in clangIndexSerialization The clangIndexSerialization target seems to be unused, so it didn't build when I ran `check-all` before landing d1f86c3d. This should resolve the deprecation warning that caused some build bots to fail. Added: Modified: clang/include/clang/IndexSerialization/SerializablePathCollection.h clang/lib/IndexSerialization/SerializablePathCollection.cpp Removed: diff --git a/clang/include/clang/IndexSerialization/SerializablePathCollection.h b/clang/include/clang/IndexSerialization/SerializablePathCollection.h index 06948db6fc95f..6337a81196688 100644 --- a/clang/include/clang/IndexSerialization/SerializablePathCollection.h +++ b/clang/include/clang/IndexSerialization/SerializablePathCollection.h @@ -110,7 +110,7 @@ class SerializablePathCollection { /// Stores path to \p FE if it hasn't been stored yet. /// \returns index to array exposed by getPathsBuffer(). - size_t tryStoreFilePath(const clang::FileEntry &FE); + size_t tryStoreFilePath(FileEntryRef FE); private: /// Stores \p Path if it is non-empty. diff --git a/clang/lib/IndexSerialization/SerializablePathCollection.cpp b/clang/lib/IndexSerialization/SerializablePathCollection.cpp index 34663738088e8..bd5f861bf482e 100644 --- a/clang/lib/IndexSerialization/SerializablePathCollection.cpp +++ b/clang/lib/IndexSerialization/SerializablePathCollection.cpp @@ -45,8 +45,8 @@ SerializablePathCollection::SerializablePathCollection( SysRootPath(Paths.addDirPath(SysRoot)), OutputFilePath(Paths.addDirPath(OutputFile)) {} -size_t SerializablePathCollection::tryStoreFilePath(const FileEntry &FE) { - auto FileIt = UniqueFiles.find(&FE); +size_t SerializablePathCollection::tryStoreFilePath(FileEntryRef FE) { + auto FileIt = UniqueFiles.find(FE); if (FileIt != UniqueFiles.end()) return FileIt->second; @@ -54,7 +54,7 @@ size_t SerializablePathCollection::tryStoreFilePath(const FileEntry &FE) { const auto FileIdx = Paths.addFilePath(Dir.Root, Dir.Path, sys::path::filename(FE.getName())); - UniqueFiles.try_emplace(&FE, FileIdx); + UniqueFiles.try_emplace(FE, FileIdx); return FileIdx; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/74782 The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 >From 6dc0ca62b2f678622538d7d9f40b73b196895956 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `Userss/blah`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b71..576b3c59253539 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 00..48f3d59421bd23 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74782 >From 6ab18edae7b86ca216848b7fcaff5e58fb3e186c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b71..576b3c59253539 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 00..48f3d59421bd23 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74782 >From 6ab18edae7b86ca216848b7fcaff5e58fb3e186c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH 1/2] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b7..576b3c5925353 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 0..48f3d59421bd2 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" >From 7437975205d4b6642d0439f6d5fa35204a88f67c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 21:43:55 -0800 Subject: [PATCH 2/2] Fix test on Windows --- clang/test/Lexer/case-insensitive-include-absolute.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index 48f3d59421bd2..da6de6acfb07c 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,13 +1,13 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t +// RUN: sed "s|DIR|%t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%t //--- header.h //--- tu.c.in #import "DIR/Header.h" -// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] -// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" -// CHECK-NEXT: | ^ -// CHECK-NEXT: | "[[PREFIX]]/header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[DIR]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +//
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74782 >From 6ab18edae7b86ca216848b7fcaff5e58fb3e186c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH 1/3] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b7..576b3c5925353 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 0..48f3d59421bd2 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" >From 7437975205d4b6642d0439f6d5fa35204a88f67c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 21:43:55 -0800 Subject: [PATCH 2/3] Fix test on Windows --- clang/test/Lexer/case-insensitive-include-absolute.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index 48f3d59421bd2..da6de6acfb07c 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,13 +1,13 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t +// RUN: sed "s|DIR|%t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%t //--- header.h //--- tu.c.in #import "DIR/Header.h" -// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] -// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" -// CHECK-NEXT: | ^ -// CHECK-NEXT: | "[[PREFIX]]/header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[DIR]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +//
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74782 >From 6ab18edae7b86ca216848b7fcaff5e58fb3e186c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH 1/4] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b7..576b3c5925353 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 0..48f3d59421bd2 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" >From 7437975205d4b6642d0439f6d5fa35204a88f67c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 21:43:55 -0800 Subject: [PATCH 2/4] Fix test on Windows --- clang/test/Lexer/case-insensitive-include-absolute.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index 48f3d59421bd2..da6de6acfb07c 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,13 +1,13 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t +// RUN: sed "s|DIR|%t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%t //--- header.h //--- tu.c.in #import "DIR/Header.h" -// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] -// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" -// CHECK-NEXT: | ^ -// CHECK-NEXT: | "[[PREFIX]]/header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[DIR]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +//
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
jansvoboda11 wrote: My first version of the test put backslashes into the source file (i.e. `#include "C:\foo\bar"`) which doesn't really work, because Clang treats those as string escape sequences. Instead of trying to replace `\` with `\\` in the test, I chose to use forward slashes, which should be fine. Interestingly, that prevented Clang to diagnose the case mismatch. That's because in `trySimplifyPath()`, any path component that differs from the "real path" component in anything else that case prevents the diagnostic. Importantly, this also applies to mismatching separators (e.g. 'C:/' in source, 'C:\' in real path). I don't think that's intended or reasonable, so I pushed a fix that allows the diagnostic to be emitted regardless of separator differences. Note that the diagnostic won't suggest fixing up the separators themselves, since those get inherited from the in-source spelling, not from the real path. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/74892 AFAICT, `ModuleFile::File` can be `std::nullopt` only for PCM files loaded from the standard input. This patch starts setting that variable to `FileManager::getSTDIN()` in that case, which makes it possible to remove the optionality, and also simplifies code that actually reads the file. This is part of an effort to get rid of `Optional{File,Directory}EntryRefDegradesTo{File,Directory}EntryPtr`. >From fb5c8c9fe856aaa2a314effa26486d5fbf019140 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 14:04:35 -0800 Subject: [PATCH] [clang] NFCI: Make `ModuleFile::File` non-optional --- .../include/clang/Serialization/ModuleFile.h | 6 +-- clang/lib/Serialization/ASTReader.cpp | 2 +- clang/lib/Serialization/ASTWriter.cpp | 2 +- clang/lib/Serialization/GlobalModuleIndex.cpp | 4 +- clang/lib/Serialization/ModuleManager.cpp | 54 --- 5 files changed, 29 insertions(+), 39 deletions(-) diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 48be8676cc26a4..70ab61dec8b6bf 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -123,8 +123,8 @@ class InputFile { /// other modules. class ModuleFile { public: - ModuleFile(ModuleKind Kind, unsigned Generation) - : Kind(Kind), Generation(Generation) {} + ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation) + : Kind(Kind), File(File), Generation(Generation) {} ~ModuleFile(); // === General information === @@ -176,7 +176,7 @@ class ModuleFile { bool DidReadTopLevelSubmodule = false; /// The file entry for the module file. - OptionalFileEntryRefDegradesToFileEntryPtr File; + FileEntryRef File; /// The signature of the module file, which may be used instead of the size /// and modification time to identify this particular file. diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index f22da838424b41..49f25d6648c801 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5786,7 +5786,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, PartialDiagnostic(diag::err_module_file_conflict, ContextObj->DiagAllocator) << CurrentModule->getTopLevelModuleName() << CurFile->getName() -<< F.File->getName(); +<< F.File.getName(); return DiagnosticError::create(CurrentImportLoc, ConflictError); } } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 6df815234e235f..38e8b8ccbe058d 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1413,7 +1413,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context, // If we have calculated signature, there is no need to store // the size or timestamp. - Record.push_back(M.Signature ? 0 : M.File->getSize()); + Record.push_back(M.Signature ? 0 : M.File.getSize()); Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File)); llvm::append_range(Record, M.Signature); diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index fb80a1998d0efe..dd4fc3e009050f 100644 --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -342,8 +342,8 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) { // If the size and modification time match what we expected, record this // module file. bool Failed = true; - if (File->File->getSize() == Info.Size && - File->File->getModificationTime() == Info.ModTime) { + if (File->File.getSize() == Info.Size && + File->File.getModificationTime() == Info.ModTime) { Info.File = File; ModulesByFile[File] = Known->second; diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index de4cd3d05853ac..7cf97125b8ef03 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -108,7 +108,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // Look for the file entry. This only fails if the expected size or // modification time differ. - OptionalFileEntryRefDegradesToFileEntryPtr Entry; + OptionalFileEntryRef Entry; if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) { // If we're not expecting to pull this file out of the module cache, it // might have a different mtime due to being moved across filesystems in @@ -123,7 +123,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, return OutOfDate; } - if (!Entry && FileName != "-") { + if (
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
@@ -441,22 +434,19 @@ void ModuleManager::visit(llvm::function_ref Visitor, bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize, time_t ExpectedModTime, OptionalFileEntryRef &File) { - File = std::nullopt; - if (FileName == "-") + if (FileName == "-") { +File = expectedToOptional(FileMgr.getSTDIN()); return false; + } // Open the file immediately to ensure there is no race between stat'ing and // opening the file. - OptionalFileEntryRef FileOrErr = - expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true, -/*CacheFailure=*/false)); - if (!FileOrErr) -return false; - - File = *FileOrErr; + File = expectedToOptional(FileMgr.getFileRef(FileName, /*OpenFile=*/true, jansvoboda11 wrote: I guess I could've replace this with `getOptionalFileRef()` while I'm touching the line. https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74892 >From fb5c8c9fe856aaa2a314effa26486d5fbf019140 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 14:04:35 -0800 Subject: [PATCH 1/2] [clang] NFCI: Make `ModuleFile::File` non-optional --- .../include/clang/Serialization/ModuleFile.h | 6 +-- clang/lib/Serialization/ASTReader.cpp | 2 +- clang/lib/Serialization/ASTWriter.cpp | 2 +- clang/lib/Serialization/GlobalModuleIndex.cpp | 4 +- clang/lib/Serialization/ModuleManager.cpp | 54 --- 5 files changed, 29 insertions(+), 39 deletions(-) diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 48be8676cc26a4..70ab61dec8b6bf 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -123,8 +123,8 @@ class InputFile { /// other modules. class ModuleFile { public: - ModuleFile(ModuleKind Kind, unsigned Generation) - : Kind(Kind), Generation(Generation) {} + ModuleFile(ModuleKind Kind, FileEntryRef File, unsigned Generation) + : Kind(Kind), File(File), Generation(Generation) {} ~ModuleFile(); // === General information === @@ -176,7 +176,7 @@ class ModuleFile { bool DidReadTopLevelSubmodule = false; /// The file entry for the module file. - OptionalFileEntryRefDegradesToFileEntryPtr File; + FileEntryRef File; /// The signature of the module file, which may be used instead of the size /// and modification time to identify this particular file. diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index f22da838424b41..49f25d6648c801 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -5786,7 +5786,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F, PartialDiagnostic(diag::err_module_file_conflict, ContextObj->DiagAllocator) << CurrentModule->getTopLevelModuleName() << CurFile->getName() -<< F.File->getName(); +<< F.File.getName(); return DiagnosticError::create(CurrentImportLoc, ConflictError); } } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 6df815234e235f..38e8b8ccbe058d 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1413,7 +1413,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context, // If we have calculated signature, there is no need to store // the size or timestamp. - Record.push_back(M.Signature ? 0 : M.File->getSize()); + Record.push_back(M.Signature ? 0 : M.File.getSize()); Record.push_back(M.Signature ? 0 : getTimestampForOutput(M.File)); llvm::append_range(Record, M.Signature); diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index fb80a1998d0efe..dd4fc3e009050f 100644 --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -342,8 +342,8 @@ bool GlobalModuleIndex::loadedModuleFile(ModuleFile *File) { // If the size and modification time match what we expected, record this // module file. bool Failed = true; - if (File->File->getSize() == Info.Size && - File->File->getModificationTime() == Info.ModTime) { + if (File->File.getSize() == Info.Size && + File->File.getModificationTime() == Info.ModTime) { Info.File = File; ModulesByFile[File] = Known->second; diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index de4cd3d05853ac..7cf97125b8ef03 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -108,7 +108,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // Look for the file entry. This only fails if the expected size or // modification time differ. - OptionalFileEntryRefDegradesToFileEntryPtr Entry; + OptionalFileEntryRef Entry; if (Type == MK_ExplicitModule || Type == MK_PrebuiltModule) { // If we're not expecting to pull this file out of the module cache, it // might have a different mtime due to being moved across filesystems in @@ -123,7 +123,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, return OutOfDate; } - if (!Entry && FileName != "-") { + if (!Entry) { ErrorStr = "module file not found"; return Missing; } @@ -150,7 +150,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, }; // Check whether we already loaded this module, before - if (ModuleFile *ModuleEntry = Modules.lookup(Entry)) { + if (ModuleFile *ModuleEntry = Modules.lookup(*Entry)) { if (implicitModuleNamesMatch(Type, ModuleEntry, *Entr
[clang] [clang] NFCI: Make `ModuleFile::File` non-optional (PR #74892)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/74892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` (PR #74899)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/74899 None >From de1d9c6808271ea802813daa71e6d94bce8a9894 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 11:38:49 -0800 Subject: [PATCH] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` --- .../ExpandModularHeadersPPCallbacks.cpp | 2 +- clang/include/clang/Basic/FileEntry.h | 66 --- clang/include/clang/Basic/Module.h| 2 +- clang/include/clang/Basic/SourceManager.h | 14 ++-- clang/include/clang/Lex/PreprocessorLexer.h | 2 +- .../include/clang/Serialization/ModuleFile.h | 2 +- clang/lib/Frontend/CompilerInstance.cpp | 2 +- clang/lib/Lex/ModuleMap.cpp | 4 +- clang/lib/Lex/PPDirectives.cpp| 3 +- clang/lib/Lex/Pragma.cpp | 2 +- clang/lib/Lex/PreprocessorLexer.cpp | 3 +- clang/lib/Serialization/ASTReader.cpp | 6 +- clang/lib/Serialization/ASTWriter.cpp | 6 +- clang/lib/Serialization/ModuleManager.cpp | 4 +- .../DependencyScanning/ModuleDepCollector.cpp | 2 +- clang/tools/libclang/CXIndexDataConsumer.cpp | 6 +- clang/tools/libclang/CXIndexDataConsumer.h| 2 +- clang/unittests/Basic/FileEntryTest.cpp | 25 --- 18 files changed, 31 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp index 52cc2e6569b052..0b1e9f59e1a70c 100644 --- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp +++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp @@ -171,7 +171,7 @@ void ExpandModularHeadersPPCallbacks::InclusionDirective( if (Imported) { serialization::ModuleFile *MF = Compiler.getASTReader()->getModuleManager().lookup( -Imported->getASTFile()); +*Imported->getASTFile()); handleModuleFile(MF); } parseToLocation(DirectiveLoc); diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h index 6351aeae92e2c4..35efa147950f06 100644 --- a/clang/include/clang/Basic/FileEntry.h +++ b/clang/include/clang/Basic/FileEntry.h @@ -279,72 +279,6 @@ template <> struct DenseMapInfo { namespace clang { -/// Wrapper around OptionalFileEntryRef that degrades to 'const FileEntry*', -/// facilitating incremental patches to propagate FileEntryRef. -/// -/// This class can be used as return value or field where it's convenient for -/// an OptionalFileEntryRef to degrade to a 'const FileEntry*'. The purpose -/// is to avoid code churn due to dances like the following: -/// \code -/// // Old code. -/// lvalue = rvalue; -/// -/// // Temporary code from an incremental patch. -/// OptionalFileEntryRef MaybeF = rvalue; -/// lvalue = MaybeF ? &MaybeF.getFileEntry() : nullptr; -/// -/// // Final code. -/// lvalue = rvalue; -/// \endcode -/// -/// FIXME: Once FileEntryRef is "everywhere" and FileEntry::LastRef and -/// FileEntry::getName have been deleted, delete this class and replace -/// instances with OptionalFileEntryRef. -class OptionalFileEntryRefDegradesToFileEntryPtr : public OptionalFileEntryRef { -public: - OptionalFileEntryRefDegradesToFileEntryPtr() = default; - OptionalFileEntryRefDegradesToFileEntryPtr( - OptionalFileEntryRefDegradesToFileEntryPtr &&) = default; - OptionalFileEntryRefDegradesToFileEntryPtr( - const OptionalFileEntryRefDegradesToFileEntryPtr &) = default; - OptionalFileEntryRefDegradesToFileEntryPtr & - operator=(OptionalFileEntryRefDegradesToFileEntryPtr &&) = default; - OptionalFileEntryRefDegradesToFileEntryPtr & - operator=(const OptionalFileEntryRefDegradesToFileEntryPtr &) = default; - - OptionalFileEntryRefDegradesToFileEntryPtr(std::nullopt_t) {} - OptionalFileEntryRefDegradesToFileEntryPtr(FileEntryRef Ref) - : OptionalFileEntryRef(Ref) {} - OptionalFileEntryRefDegradesToFileEntryPtr(OptionalFileEntryRef MaybeRef) - : OptionalFileEntryRef(MaybeRef) {} - - OptionalFileEntryRefDegradesToFileEntryPtr &operator=(std::nullopt_t) { -OptionalFileEntryRef::operator=(std::nullopt); -return *this; - } - OptionalFileEntryRefDegradesToFileEntryPtr &operator=(FileEntryRef Ref) { -OptionalFileEntryRef::operator=(Ref); -return *this; - } - OptionalFileEntryRefDegradesToFileEntryPtr & - operator=(OptionalFileEntryRef MaybeRef) { -OptionalFileEntryRef::operator=(MaybeRef); -return *this; - } - - /// Degrade to 'const FileEntry *' to allow FileEntry::LastRef and - /// FileEntry::getName have been deleted, delete this class and replace - /// instances with OptionalFileEntryRef - operator const FileEntry *() const { -return has_value() ? &(*this)->getFileEntry() : nullptr; - } -}; - -static_assert( -std::is_trivially_copyable< -OptionalFileEntryRefDegradesToFileEntryPtr>::value, -"OptionalFileEntryRefDegr
[clang] [clang-tools-extra] [clang] NFC: Remove `OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr` (PR #74900)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/74900 None >From 999dcc9b5237af4cca7460042edb709af6ccdf05 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 14:22:04 -0800 Subject: [PATCH] [clang] NFC: Remove 'OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr' --- .../clang-tidy/misc/IncludeCleanerCheck.cpp | 2 +- clang-tools-extra/clangd/IncludeCleaner.cpp | 8 +- .../include-cleaner/lib/Analysis.cpp | 6 +- clang/include/clang/Basic/DirectoryEntry.h| 74 --- clang/include/clang/Basic/Module.h| 2 +- clang/include/clang/Lex/ModuleMap.h | 14 ++-- clang/lib/Serialization/ASTReader.cpp | 2 +- 7 files changed, 15 insertions(+), 93 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp index e336ba1ee1fa72..5ae6caedb7f4c0 100644 --- a/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp @@ -124,7 +124,7 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) { MainFileDecls.push_back(D); } llvm::DenseSet SeenSymbols; - const DirectoryEntry *ResourceDir = + OptionalDirectoryEntryRef ResourceDir = PP->getHeaderSearchInfo().getModuleMap().getBuiltinDir(); // FIXME: Find a way to have less code duplication between include-cleaner // analysis implementation and the below code. diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index b0a3c290bad660..dda7c9f581f69c 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -397,10 +397,10 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) { std::vector MissingIncludes; llvm::DenseSet Used; trace::Span Tracer("include_cleaner::walkUsed"); - const DirectoryEntry *ResourceDir = AST.getPreprocessor() -.getHeaderSearchInfo() -.getModuleMap() -.getBuiltinDir(); + OptionalDirectoryEntryRef ResourceDir = AST.getPreprocessor() + .getHeaderSearchInfo() + .getModuleMap() + .getBuiltinDir(); include_cleaner::walkUsed( AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros, AST.getPragmaIncludes().get(), AST.getPreprocessor(), diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp index 09365c36f9f2c5..450c4c796c1415 100644 --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -87,7 +87,7 @@ analyze(llvm::ArrayRef ASTRoots, llvm::StringSet<> Missing; if (!HeaderFilter) HeaderFilter = [](llvm::StringRef) { return false; }; - const DirectoryEntry *ResourceDir = + OptionalDirectoryEntryRef ResourceDir = PP.getHeaderSearchInfo().getModuleMap().getBuiltinDir(); walkUsed(ASTRoots, MacroRefs, PI, PP, [&](const SymbolReference &Ref, llvm::ArrayRef Providers) { @@ -95,7 +95,7 @@ analyze(llvm::ArrayRef ASTRoots, for (const Header &H : Providers) { if (H.kind() == Header::Physical && (H.physical() == MainFile || -H.physical().getDir() == ResourceDir)) { +(ResourceDir && H.physical().getDir() == *ResourceDir))) { Satisfied = true; } for (const Include *I : Inc.match(H)) { @@ -114,7 +114,7 @@ analyze(llvm::ArrayRef ASTRoots, for (const Include &I : Inc.all()) { if (Used.contains(&I) || !I.Resolved || HeaderFilter(I.Resolved->getFileEntry().tryGetRealPathName()) || -I.Resolved->getFileEntry().getDir() == ResourceDir) +(ResourceDir && I.Resolved->getFileEntry().getDir() == *ResourceDir)) continue; if (PI) { if (PI->shouldKeep(*I.Resolved)) diff --git a/clang/include/clang/Basic/DirectoryEntry.h b/clang/include/clang/Basic/DirectoryEntry.h index 5d083e68facd7a..906c2e9af23b31 100644 --- a/clang/include/clang/Basic/DirectoryEntry.h +++ b/clang/include/clang/Basic/DirectoryEntry.h @@ -245,78 +245,4 @@ template <> struct DenseMapInfo { } // end namespace llvm -namespace clang { - -/// Wrapper around OptionalDirectoryEntryRef that degrades to 'const -/// DirectoryEntry*', facilitating incremental patches to propagate -/// DirectoryEntryRef. -/// -/// This class can be used as return value or field where it's convenient for -/// an OptionalDirectoryEntryRef to degrade to a 'const DirectoryEntry*'. The -/// purpose is to avoid code churn due to dances like the following: -/// \code -/// // Old code. -/// lvalue = rvalue; -/// -/// // Temporary cod
[clang] [clang-tools-extra] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` (PR #74899)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74899 >From de1d9c6808271ea802813daa71e6d94bce8a9894 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 11:38:49 -0800 Subject: [PATCH 1/2] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` --- .../ExpandModularHeadersPPCallbacks.cpp | 2 +- clang/include/clang/Basic/FileEntry.h | 66 --- clang/include/clang/Basic/Module.h| 2 +- clang/include/clang/Basic/SourceManager.h | 14 ++-- clang/include/clang/Lex/PreprocessorLexer.h | 2 +- .../include/clang/Serialization/ModuleFile.h | 2 +- clang/lib/Frontend/CompilerInstance.cpp | 2 +- clang/lib/Lex/ModuleMap.cpp | 4 +- clang/lib/Lex/PPDirectives.cpp| 3 +- clang/lib/Lex/Pragma.cpp | 2 +- clang/lib/Lex/PreprocessorLexer.cpp | 3 +- clang/lib/Serialization/ASTReader.cpp | 6 +- clang/lib/Serialization/ASTWriter.cpp | 6 +- clang/lib/Serialization/ModuleManager.cpp | 4 +- .../DependencyScanning/ModuleDepCollector.cpp | 2 +- clang/tools/libclang/CXIndexDataConsumer.cpp | 6 +- clang/tools/libclang/CXIndexDataConsumer.h| 2 +- clang/unittests/Basic/FileEntryTest.cpp | 25 --- 18 files changed, 31 insertions(+), 122 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp index 52cc2e6569b05..0b1e9f59e1a70 100644 --- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp +++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp @@ -171,7 +171,7 @@ void ExpandModularHeadersPPCallbacks::InclusionDirective( if (Imported) { serialization::ModuleFile *MF = Compiler.getASTReader()->getModuleManager().lookup( -Imported->getASTFile()); +*Imported->getASTFile()); handleModuleFile(MF); } parseToLocation(DirectiveLoc); diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h index 6351aeae92e2c..35efa147950f0 100644 --- a/clang/include/clang/Basic/FileEntry.h +++ b/clang/include/clang/Basic/FileEntry.h @@ -279,72 +279,6 @@ template <> struct DenseMapInfo { namespace clang { -/// Wrapper around OptionalFileEntryRef that degrades to 'const FileEntry*', -/// facilitating incremental patches to propagate FileEntryRef. -/// -/// This class can be used as return value or field where it's convenient for -/// an OptionalFileEntryRef to degrade to a 'const FileEntry*'. The purpose -/// is to avoid code churn due to dances like the following: -/// \code -/// // Old code. -/// lvalue = rvalue; -/// -/// // Temporary code from an incremental patch. -/// OptionalFileEntryRef MaybeF = rvalue; -/// lvalue = MaybeF ? &MaybeF.getFileEntry() : nullptr; -/// -/// // Final code. -/// lvalue = rvalue; -/// \endcode -/// -/// FIXME: Once FileEntryRef is "everywhere" and FileEntry::LastRef and -/// FileEntry::getName have been deleted, delete this class and replace -/// instances with OptionalFileEntryRef. -class OptionalFileEntryRefDegradesToFileEntryPtr : public OptionalFileEntryRef { -public: - OptionalFileEntryRefDegradesToFileEntryPtr() = default; - OptionalFileEntryRefDegradesToFileEntryPtr( - OptionalFileEntryRefDegradesToFileEntryPtr &&) = default; - OptionalFileEntryRefDegradesToFileEntryPtr( - const OptionalFileEntryRefDegradesToFileEntryPtr &) = default; - OptionalFileEntryRefDegradesToFileEntryPtr & - operator=(OptionalFileEntryRefDegradesToFileEntryPtr &&) = default; - OptionalFileEntryRefDegradesToFileEntryPtr & - operator=(const OptionalFileEntryRefDegradesToFileEntryPtr &) = default; - - OptionalFileEntryRefDegradesToFileEntryPtr(std::nullopt_t) {} - OptionalFileEntryRefDegradesToFileEntryPtr(FileEntryRef Ref) - : OptionalFileEntryRef(Ref) {} - OptionalFileEntryRefDegradesToFileEntryPtr(OptionalFileEntryRef MaybeRef) - : OptionalFileEntryRef(MaybeRef) {} - - OptionalFileEntryRefDegradesToFileEntryPtr &operator=(std::nullopt_t) { -OptionalFileEntryRef::operator=(std::nullopt); -return *this; - } - OptionalFileEntryRefDegradesToFileEntryPtr &operator=(FileEntryRef Ref) { -OptionalFileEntryRef::operator=(Ref); -return *this; - } - OptionalFileEntryRefDegradesToFileEntryPtr & - operator=(OptionalFileEntryRef MaybeRef) { -OptionalFileEntryRef::operator=(MaybeRef); -return *this; - } - - /// Degrade to 'const FileEntry *' to allow FileEntry::LastRef and - /// FileEntry::getName have been deleted, delete this class and replace - /// instances with OptionalFileEntryRef - operator const FileEntry *() const { -return has_value() ? &(*this)->getFileEntry() : nullptr; - } -}; - -static_assert( -std::is_trivially_copyable< -OptionalFileEntryRefDegradesToFileEntryPtr>::value, -"OptionalFileEntryRefDegradesTo
[clang-tools-extra] [clang] [clang] NFC: Remove `OptionalFileEntryRefDegradesToFileEntryPtr` (PR #74899)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/74899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang] NFC: Remove `OptionalDirectoryEntryRefDegradesToDirectoryEntryPtr` (PR #74900)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/74900 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] NFC: Remove `{File, Directory}Entry::getName()` (PR #74910)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/74910 None >From f1a4ff8dc30b755e95fcd4871eb59b0d49f86c7b Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 09:29:14 -0800 Subject: [PATCH] [clang] NFC: Remove `{File,Directory}Entry::getName()` --- clang/include/clang/Basic/DirectoryEntry.h| 7 - clang/include/clang/Basic/FileEntry.h | 10 --- clang/lib/Basic/FileManager.cpp | 26 --- clang/unittests/Basic/FileManagerTest.cpp | 6 - llvm/include/llvm/Support/VirtualFileSystem.h | 3 --- llvm/lib/Support/VirtualFileSystem.cpp| 1 - .../Support/VirtualFileSystemTest.cpp | 18 - 7 files changed, 71 deletions(-) diff --git a/clang/include/clang/Basic/DirectoryEntry.h b/clang/include/clang/Basic/DirectoryEntry.h index 906c2e9af23b3..35fe529ba79df 100644 --- a/clang/include/clang/Basic/DirectoryEntry.h +++ b/clang/include/clang/Basic/DirectoryEntry.h @@ -41,13 +41,6 @@ class DirectoryEntry { DirectoryEntry &operator=(const DirectoryEntry &) = delete; friend class FileManager; friend class FileEntryTestHelper; - - // FIXME: We should not be storing a directory entry name here. - StringRef Name; // Name of the directory. - -public: - LLVM_DEPRECATED("Use DirectoryEntryRef::getName() instead.", "") - StringRef getName() const { return Name; } }; /// A reference to a \c DirectoryEntry that includes the name of the directory diff --git a/clang/include/clang/Basic/FileEntry.h b/clang/include/clang/Basic/FileEntry.h index 35efa147950f0..68d4bf6093003 100644 --- a/clang/include/clang/Basic/FileEntry.h +++ b/clang/include/clang/Basic/FileEntry.h @@ -318,18 +318,8 @@ class FileEntry { /// The file content, if it is owned by the \p FileEntry. std::unique_ptr Content; - // First access name for this FileEntry. - // - // This is Optional only to allow delayed construction (FileEntryRef has no - // default constructor). It should always have a value in practice. - // - // TODO: remove this once everyone that needs a name uses FileEntryRef. - OptionalFileEntryRef LastRef; - public: ~FileEntry(); - LLVM_DEPRECATED("Use FileEntryRef::getName() instead.", "") - StringRef getName() const { return LastRef->getName(); } StringRef tryGetRealPathName() const { return RealPathName; } off_t getSize() const { return Size; } diff --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp index d16626b106521..e715e69f7bdbb 100644 --- a/clang/lib/Basic/FileManager.cpp +++ b/clang/lib/Basic/FileManager.cpp @@ -107,7 +107,6 @@ void FileManager::addAncestorsAsVirtualDirs(StringRef Path) { // Add the virtual directory to the cache. auto *UDE = new (DirsAlloc.Allocate()) DirectoryEntry(); - UDE->Name = NamedDirEnt.first(); NamedDirEnt.second = *UDE; VirtualDirectoryEntries.push_back(UDE); @@ -179,7 +178,6 @@ FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) { // We don't have this directory yet, add it. We use the string // key from the SeenDirEntries map as the string. UDE = new (DirsAlloc.Allocate()) DirectoryEntry(); -UDE->Name = InterndDirName; } NamedDirEnt.second = *UDE; @@ -324,32 +322,10 @@ FileManager::getFileRef(StringRef Filename, bool openFile, bool CacheFailure) { FileEntryRef ReturnedRef(*NamedFileEnt); if (ReusingEntry) { // Already have an entry with this inode, return it. - -// FIXME: This hack ensures that `getDir()` will use the path that was -// used to lookup this file, even if we found a file by different path -// first. This is required in order to find a module's structure when its -// headers/module map are mapped in the VFS. -// -// See above for how this will eventually be removed. `IsVFSMapped` -// *cannot* be narrowed to `ExposesExternalVFSPath` as crash reproducers -// also depend on this logic and they have `use-external-paths: false`. -if (&DirInfo.getDirEntry() != UFE->Dir && Status.IsVFSMapped) - UFE->Dir = &DirInfo.getDirEntry(); - -// Always update LastRef to the last name by which a file was accessed. -// FIXME: Neither this nor always using the first reference is correct; we -// want to switch towards a design where we return a FileName object that -// encapsulates both the name by which the file was accessed and the -// corresponding FileEntry. -// FIXME: LastRef should be removed from FileEntry once all clients adopt -// FileEntryRef. -UFE->LastRef = ReturnedRef; - return ReturnedRef; } // Otherwise, we don't have this file yet, add it. - UFE->LastRef = ReturnedRef; UFE->Size = Status.getSize(); UFE->ModTime = llvm::sys::toTimeT(Status.getLastModificationTime()); UFE->Dir = &DirInfo.getDirEntry(); @@ -461,7 +437,6 @@ FileEntryRef FileManager::getVirtualFileRef(StringRef Filename, off_t Size, } NamedFileEnt.second = FileEntryRe
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
@@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} jansvoboda11 wrote: The LLVM Coding Standards encourage keeping consistency with the `if` block in this case and use braces. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
@@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { jansvoboda11 wrote: I find the code easier to think about with `if (Path.size() == 1) { x } else { y }` compared to `if (Path.size() != 1) { y; continue; } x` to be honest: * The `Path.size() == 1` condition is more specific than `Path.size() != 1`. * To think about the path to `x`, you don't have to do double negation and notice the early `continue`. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74782 >From 6ab18edae7b86ca216848b7fcaff5e58fb3e186c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH 1/5] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b71..576b3c59253539 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 00..48f3d59421bd23 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" >From 7437975205d4b6642d0439f6d5fa35204a88f67c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 21:43:55 -0800 Subject: [PATCH 2/5] Fix test on Windows --- clang/test/Lexer/case-insensitive-include-absolute.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index 48f3d59421bd23..da6de6acfb07c7 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,13 +1,13 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t +// RUN: sed "s|DIR|%t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%t //--- header.h //--- tu.c.in #import "DIR/Header.h" -// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] -// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" -// CHECK-NEXT: | ^ -// CHECK-NEXT: | "[[PREFIX]]/header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[DIR]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-pat
[llvm] [clang] [clang] NFC: Remove `{File, Directory}Entry::getName()` (PR #74910)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/74910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang] NFC: Remove `{File, Directory}Entry::getName()` (PR #74910)
jansvoboda11 wrote: Note: I'd like to merge this PR after we branch off for Clang 18 in January or February 2024. I had the patch lying around, so I figured I might as well create the PR. https://github.com/llvm/llvm-project/pull/74910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/74782 >From 6ab18edae7b86ca216848b7fcaff5e58fb3e186c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 15:15:16 -0800 Subject: [PATCH 1/6] [clang][lex] Fix non-portability diagnostics with absolute path The existing code incorrectly assumes that `Path` can be empty. It can't, it always contains at least `<` or `"`. On Unix, this patch fixes an incorrect diagnostics that instead of `"/Users/blah"` suggested `"Userss/blah"`. In assert builds, this would outright crash. rdar://91172342 --- clang/lib/Lex/PPDirectives.cpp| 22 --- .../Lexer/case-insensitive-include-absolute.c | 13 +++ 2 files changed, 27 insertions(+), 8 deletions(-) create mode 100644 clang/test/Lexer/case-insensitive-include-absolute.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 956e2276f25b71..576b3c59253539 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { +// If the first entry in Components is a directory separator, +// then the code at the bottom of this loop that keeps the original +// directory separator style copies it. + } else { +// If the second entry is a directory separator (the C:\ case), +// then that separator already got copied when the C: was processed +// and we want to skip that entry. +continue; + } +} else { Path.append(Component); -else if (!Path.empty()) - continue; +} // Append the separator(s) the user used, or the close quote if (Path.size() > NameWithoriginalSlashes.size()) { diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c new file mode 100644 index 00..48f3d59421bd23 --- /dev/null +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -0,0 +1,13 @@ +// REQUIRES: case-insensitive-filesystem + +// RUN: rm -rf %t && split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t + +//--- header.h +//--- tu.c.in +#import "DIR/Header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] +// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" +// CHECK-NEXT: | ^ +// CHECK-NEXT: | "[[PREFIX]]/header.h" >From 7437975205d4b6642d0439f6d5fa35204a88f67c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 7 Dec 2023 21:43:55 -0800 Subject: [PATCH 2/6] Fix test on Windows --- clang/test/Lexer/case-insensitive-include-absolute.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index 48f3d59421bd23..da6de6acfb07c7 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,13 +1,13 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DPREFIX=%t +// RUN: sed "s|DIR|%t|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%t //--- header.h //--- tu.c.in #import "DIR/Header.h" -// CHECK: tu.c:1:9: warning: non-portable path to file '"[[PREFIX]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-path] -// CHECK-NEXT:1 | #import "[[PREFIX]]/Header.h" -// CHECK-NEXT: | ^ -// CHECK-NEXT: | "[[PREFIX]]/header.h" +// CHECK: tu.c:1:9: warning: non-portable path to file '"[[DIR]]/header.h"'; specified path differs in case from file name on disk [-Wnonportable-include-pat
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
@@ -2466,15 +2466,21 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( // The drive letter is optional for absolute paths on Windows, but // clang currently cannot process absolute paths in #include lines that // don't have a drive. -// If the first entry in Components is a directory separator, -// then the code at the bottom of this loop that keeps the original -// directory separator style copies it. If the second entry is -// a directory separator (the C:\ case), then that separator already -// got copied when the C: was processed and we want to skip that entry. -if (!(Component.size() == 1 && IsSep(Component[0]))) +if (Component.size() == 1 && IsSep(Component[0])) { + // Note: Path always contains at least '<' or '"'. + if (Path.size() == 1) { jansvoboda11 wrote: Fine, I left the code structure as it was before and just fixed the faulty condition. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangModule] Fix decl-params-determinisim test after serialization change (PR #72572)
@@ -23,27 +23,28 @@ jansvoboda11 wrote: Should `op13` in the comment be updated? https://github.com/llvm/llvm-project/pull/72572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ClangModule] Fix decl-params-determinisim test after serialization change (PR #72572)
https://github.com/jansvoboda11 approved this pull request. LGTM with little nit. https://github.com/llvm/llvm-project/pull/72572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Don't prevent translation of FW_Private includes when explicitly building FW (PR #70714)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/70714 We prevent translating `#include ` into an import of FW_Private when compiling the implementation of FW or FW_Private. This is specified via `-fmodule-name=` on the TU command line (used to be `-fmodule-implementation-of`). This logic is supposed to only kick in when imported directly from a TU, but it currently also kicks in when compiling the public FW module explicitly (since it also has `-fmodule-name=` on the command line). This patch makes sure this logic only kicks in for the case that used to be `-fmodule-implementation-of` (for the TU), and not for all `-fmodule-name=` cases (especially for the explicit compile of a module). rdar://101051277; related: rdar://37500098&38434694 >From 8c2bbe27909fa6dbf0aac2c2b856a30e72046b2f Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 30 Oct 2023 11:44:56 -0700 Subject: [PATCH] [clang][modules] Don't prevent translation of FW_Private includes when explicitly building FW We prevent translating `#include ` into an import of FW_Private when compiling the implementation of FW or FW_Private. This is specified via `-fmodule-name=` on the TU command line (used to be `-fmodule-implementation-of`). This logic is supposed to only kick in when imported directly from a TU, but it currently also kicks in when compiling the public FW module explicitly (since it also has `-fmodule-name=` on the command line). This patch makes sure this logic only kicks in for the case that used to be `-fmodule-implementation-of` (for the TU), and not for all `-fmodule-name=` cases (especially for the explicit compile of a module). rdar://101051277; related: rdar://37500098&38434694 --- clang/lib/Basic/Module.cpp| 7 +- .../ClangScanDeps/modules-priv-fw-from-pub.m | 121 ++ 2 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 clang/test/ClangScanDeps/modules-priv-fw-from-pub.m diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index cc2e5be98d32d34..e4ac1abf12a7f8e 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -161,9 +161,10 @@ bool Module::isForBuilding(const LangOptions &LangOpts) const { StringRef TopLevelName = getTopLevelModuleName(); StringRef CurrentModule = LangOpts.CurrentModule; - // When building framework Foo, we want to make sure that Foo *and* - // Foo_Private are textually included and no modules are built for both. - if (getTopLevelModule()->IsFramework && + // When building the implementation of framework Foo, we want to make sure + // that Foo *and* Foo_Private are textually included and no modules are built + // for either. + if (!LangOpts.isCompilingModule() && getTopLevelModule()->IsFramework && CurrentModule == LangOpts.ModuleName && !CurrentModule.endswith("_Private") && TopLevelName.endswith("_Private")) TopLevelName = TopLevelName.drop_back(8); diff --git a/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m b/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m new file mode 100644 index 000..cb82e0c366322c9 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-priv-fw-from-pub.m @@ -0,0 +1,121 @@ +// This test checks that importing private headers from the public headers of +// a framework is consistent between the dependency scanner and the explicit build. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +//--- frameworks/FW.framework/Modules/module.modulemap +framework module FW { header "FW.h" } +//--- frameworks/FW.framework/Modules/module.private.modulemap +framework module FW_Private { header "FW_Private.h"} +//--- frameworks/FW.framework/Headers/FW.h +#include +//--- frameworks/FW.framework/PrivateHeaders/FW_Private.h +@import Dependency; + +//--- modules/module.modulemap +module Dependency { header "dependency.h" } +//--- modules/dependency.h +// empty + +//--- tu.m +#include + +//--- cdb.json.in +[{ + "file": "DIR/tu.m", + "directory": "DIR", + "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fimplicit-module-maps -I DIR/modules -F DIR/frameworks -Wno-framework-include-private-from-public -Wno-atimport-in-framework-header -c DIR/tu.m -o DIR/tu.o" +}] + +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/deps.json + +// Check that FW is reported to depend on FW_Private. +// RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t +// CHECK: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/modules/module.modulemap", +// CHECK-NEXT: "command-line": [ +// CHECK:], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK-NEXT: "[[PREFIX]]/modules/dependency.h", +// CHECK-NEXT: "[[
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/69975 >From 0270c76e779457486ee89f100db2b7151832c290 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 10 Oct 2023 14:16:13 -0700 Subject: [PATCH 1/5] [clang][modules] Make `DIAGNOSTIC_OPTIONS` skippable --- clang/include/clang/Driver/Options.td | 4 ++ clang/include/clang/Lex/HeaderSearchOptions.h | 6 ++- clang/lib/Serialization/ASTWriter.cpp | 4 ++ .../Modules/diagnostic-options-mismatch.c | 41 +++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 clang/test/Modules/diagnostic-options-mismatch.c diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 5415b18d3f406df..68ec32bdf56a555 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2944,6 +2944,10 @@ def fno_modules_validate_textual_header_includes : MarshallingInfoNegativeFlag>, HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. " "This flag will be removed in a future Clang release.">; +defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options", +HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse, +PosFlag, +NegFlag, BothFlags<[], [CC1Option]>>; def fincremental_extensions : Flag<["-"], "fincremental-extensions">, diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index c7d95006bb779ad..d6dd94fe9466299 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -219,6 +219,9 @@ class HeaderSearchOptions { unsigned ModulesValidateDiagnosticOptions : 1; + /// Whether to entirely skip writing diagnostic options. + unsigned ModulesSkipDiagnosticOptions : 1; + unsigned ModulesHashContent : 1; /// Whether we should include all things that could impact the module in the @@ -238,7 +241,8 @@ class HeaderSearchOptions { ModulesValidateSystemHeaders(false), ValidateASTInputFilesContent(false), ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false), -ModulesValidateDiagnosticOptions(true), ModulesHashContent(false), +ModulesValidateDiagnosticOptions(true), +ModulesSkipDiagnosticOptions(false), ModulesHashContent(false), ModulesStrictContextHash(false) {} /// AddPath - Add the \p Path path to the specified \p Group list. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 27700c711d52fdd..e063f8d6acc295a 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1215,6 +1215,9 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // Diagnostic options. const auto &Diags = Context.getDiagnostics(); const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions(); + if (!PP.getHeaderSearchInfo() + .getHeaderSearchOpts() + .ModulesSkipDiagnosticOptions) { #define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name); #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ Record.push_back(static_cast(DiagOpts.get##Name())); @@ -1229,6 +1232,7 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // are generally transient files and will almost always be overridden. Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record); Record.clear(); + } // Header search paths. Record.clear(); diff --git a/clang/test/Modules/diagnostic-options-mismatch.c b/clang/test/Modules/diagnostic-options-mismatch.c new file mode 100644 index 000..cdd6f897729a9d8 --- /dev/null +++ b/clang/test/Modules/diagnostic-options-mismatch.c @@ -0,0 +1,41 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: split-file %s %t + +//--- module.modulemap +module Mod { header "mod.h" } +//--- mod.h +//--- tu.c +#include "mod.h" + +// Without any extra compiler flags, mismatched diagnostic options trigger recompilation of modules. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -Wnon-modular-include-in-module +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DID-REBUILD +// DID-REBUILD: remark: building module 'Mod' + +// When skipping serialization of diagnostic options, mismatches cannot be detected, old PCM file gets reused. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Wnon-modular-include-in-module +/
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
@@ -1212,52 +1212,54 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, Record.clear(); } + const auto &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); + // Diagnostic options. const auto &Diags = Context.getDiagnostics(); const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions(); + if (!HSOpts.ModulesSkipDiagnosticOptions) { #define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name); #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ Record.push_back(static_cast(DiagOpts.get##Name())); jansvoboda11 wrote: `clang-format` unfortunately does not indent this line. https://github.com/llvm/llvm-project/pull/69975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record (PR #70874)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/70874 None >From c3602bceb01aa93f801670a31bb43903d6a10d9c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 31 Oct 2023 16:35:38 -0700 Subject: [PATCH] [clang][deps] Skip writing `DIAG_PRAGMA_MAPPINGS` record --- clang/include/clang/Lex/HeaderSearchOptions.h | 4 clang/lib/Serialization/ASTWriter.cpp | 4 ++-- .../Tooling/DependencyScanning/DependencyScanningWorker.cpp | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index c7d95006bb779ad..e7204bf718b5f75 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -219,6 +219,10 @@ class HeaderSearchOptions { unsigned ModulesValidateDiagnosticOptions : 1; + /// Whether to entirely skip writing pragma diagnostic mappings. + /// Primarily used to speed up deserialization during dependency scanning. + unsigned ModulesSkipPragmaDiagnosticMappings : 1; + unsigned ModulesHashContent : 1; /// Whether we should include all things that could impact the module in the diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 739344b9a128dcf..011a94c5b55520d 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1259,8 +1259,8 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, Stream.EmitRecord(HEADER_SEARCH_PATHS, Record); - // Write out the diagnostic/pragma mappings. - WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule); + if (!HSOpts.ModulesSkipPragmaDiagnosticMappings) +WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule); // Header search entry usage. auto HSEntryUsage = PP.getHeaderSearchInfo().computeUserEntryUsage(); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 29df0c3a0afdb5c..45cd247148dd34d 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -252,6 +252,8 @@ class DependencyScanningAction : public tooling::ToolAction { // TODO: Implement diagnostic bucketing to reduce the impact of strict // context hashing. ScanInstance.getHeaderSearchOpts().ModulesStrictContextHash = true; +ScanInstance.getHeaderSearchOpts().ModulesSkipPragmaDiagnosticMappings = +true; // Avoid some checks and module map parsing when loading PCM files. ScanInstance.getPreprocessorOpts().ModulesCheckRelocated = false; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Don't prevent translation of FW_Private includes when explicitly building FW (PR #70714)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/70714 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Track included files per submodule (PR #71117)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/71117 None >From 9debc58d5135fbde51967dfb076d0ec5d954df38 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 2 Nov 2023 14:35:30 -0700 Subject: [PATCH] [clang][modules] Track included files per submodule --- clang/include/clang/Basic/Module.h | 2 + clang/include/clang/Lex/Preprocessor.h | 39 +++--- clang/lib/Serialization/ASTReader.cpp| 52 ++- clang/lib/Serialization/ASTReaderInternals.h | 3 +- clang/lib/Serialization/ASTWriter.cpp| 39 +++--- clang/test/Modules/per-submodule-includes.m | 53 6 files changed, 149 insertions(+), 39 deletions(-) create mode 100644 clang/test/Modules/per-submodule-includes.m diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 6a7423938bdb8fa..41f3b4fb46423d6 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -423,6 +423,8 @@ class alignas(8) Module { /// to import but didn't because they are not direct uses. llvm::SmallSetVector UndeclaredUses; + llvm::DenseSet Includes; + /// A library or framework to link against when an entity from this /// module is used. struct LinkLibrary { diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 4a99447e757c6ac..fef608efdee09d9 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -982,6 +982,8 @@ class Preprocessor { /// The set of modules that are visible within the submodule. VisibleModuleSet VisibleModules; +/// The files that have been included. +IncludedFilesSet IncludedFiles; // FIXME: CounterValue? // FIXME: PragmaPushMacroInfo? }; @@ -994,8 +996,8 @@ class Preprocessor { /// in a submodule. SubmoduleState *CurSubmoduleState; - /// The files that have been included. - IncludedFilesSet IncludedFiles; + /// The files that have been included outside of (sub)modules. + IncludedFilesSet Includes; /// The set of top-level modules that affected preprocessing, but were not /// imported. @@ -1484,19 +1486,40 @@ class Preprocessor { /// Mark the file as included. /// Returns true if this is the first time the file was included. bool markIncluded(FileEntryRef File) { -HeaderInfo.getFileInfo(File); -return IncludedFiles.insert(File).second; +bool AlreadyIncluded = alreadyIncluded(File); +CurSubmoduleState->IncludedFiles.insert(File); +if (!BuildingSubmoduleStack.empty()) + BuildingSubmoduleStack.back().M->Includes.insert(File); +else if (Module *M = getCurrentModule()) + M->Includes.insert(File); +else + Includes.insert(File); +return !AlreadyIncluded; } /// Return true if this header has already been included. bool alreadyIncluded(FileEntryRef File) const { HeaderInfo.getFileInfo(File); -return IncludedFiles.count(File); +if (CurSubmoduleState->IncludedFiles.contains(File)) + return true; +// TODO: Do this more efficiently. +for (const auto &[Name, M] : HeaderInfo.getModuleMap().modules()) + if (CurSubmoduleState->VisibleModules.isVisible(M)) +if (M->Includes.contains(File)) + return true; +return false; + } + + void markIncludedOnTopLevel(const FileEntry *File) { +Includes.insert(File); +CurSubmoduleState->IncludedFiles.insert(File); + } + + void markIncludedInModule(Module *M, const FileEntry *File) { +M->Includes.insert(File); } - /// Get the set of included files. - IncludedFilesSet &getIncludedFiles() { return IncludedFiles; } - const IncludedFilesSet &getIncludedFiles() const { return IncludedFiles; } + const IncludedFilesSet &getTopLevelIncludes() const { return Includes; } /// Return the name of the macro defined before \p Loc that has /// spelling \p Tokens. If there are multiple macros with same spelling, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 42b48d230af7a97..9c32dcf3d954761 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -1975,19 +1975,15 @@ ASTReader::getGlobalPreprocessedEntityID(ModuleFile &M, return LocalID + I->second; } -const FileEntry *HeaderFileInfoTrait::getFile(const internal_key_type &Key) { +OptionalFileEntryRefDegradesToFileEntryPtr +HeaderFileInfoTrait::getFile(const internal_key_type &Key) { FileManager &FileMgr = Reader.getFileManager(); - if (!Key.Imported) { -if (auto File = FileMgr.getFile(Key.Filename)) - return *File; -return nullptr; - } + if (!Key.Imported) +return FileMgr.getOptionalFileRef(Key.Filename); std::string Resolved = std::string(Key.Filename); Reader.ResolveImportedPath(M, Resolved); - if (auto File = FileMgr.getFile(Resolved)) -return *File; - return nullptr; + return Fil
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/69975 >From 5dd9d64726dba95f71dfb276dd1be4d386313a99 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 10 Oct 2023 14:16:13 -0700 Subject: [PATCH 1/6] [clang][modules] Make `DIAGNOSTIC_OPTIONS` skippable --- clang/include/clang/Driver/Options.td | 4 ++ clang/include/clang/Lex/HeaderSearchOptions.h | 6 ++- clang/lib/Serialization/ASTWriter.cpp | 4 ++ .../Modules/diagnostic-options-mismatch.c | 41 +++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 clang/test/Modules/diagnostic-options-mismatch.c diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index fcf6a4b2ccb2369..b34c68f7f238749 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -2947,6 +2947,10 @@ def fno_modules_validate_textual_header_includes : MarshallingInfoNegativeFlag>, HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. " "This flag will be removed in a future Clang release.">; +defm modules_skip_diagnostic_options : BoolFOption<"modules-skip-diagnostic-options", +HeaderSearchOpts<"ModulesSkipDiagnosticOptions">, DefaultFalse, +PosFlag, +NegFlag, BothFlags<[], [CC1Option]>>; def fincremental_extensions : Flag<["-"], "fincremental-extensions">, diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h index c7d95006bb779ad..d6dd94fe9466299 100644 --- a/clang/include/clang/Lex/HeaderSearchOptions.h +++ b/clang/include/clang/Lex/HeaderSearchOptions.h @@ -219,6 +219,9 @@ class HeaderSearchOptions { unsigned ModulesValidateDiagnosticOptions : 1; + /// Whether to entirely skip writing diagnostic options. + unsigned ModulesSkipDiagnosticOptions : 1; + unsigned ModulesHashContent : 1; /// Whether we should include all things that could impact the module in the @@ -238,7 +241,8 @@ class HeaderSearchOptions { ModulesValidateSystemHeaders(false), ValidateASTInputFilesContent(false), ForceCheckCXX20ModulesInputFiles(false), UseDebugInfo(false), -ModulesValidateDiagnosticOptions(true), ModulesHashContent(false), +ModulesValidateDiagnosticOptions(true), +ModulesSkipDiagnosticOptions(false), ModulesHashContent(false), ModulesStrictContextHash(false) {} /// AddPath - Add the \p Path path to the specified \p Group list. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index ab70594607530f4..c63f66f551b0eb6 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1215,6 +1215,9 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // Diagnostic options. const auto &Diags = Context.getDiagnostics(); const DiagnosticOptions &DiagOpts = Diags.getDiagnosticOptions(); + if (!PP.getHeaderSearchInfo() + .getHeaderSearchOpts() + .ModulesSkipDiagnosticOptions) { #define DIAGOPT(Name, Bits, Default) Record.push_back(DiagOpts.Name); #define ENUM_DIAGOPT(Name, Type, Bits, Default) \ Record.push_back(static_cast(DiagOpts.get##Name())); @@ -1229,6 +1232,7 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP, // are generally transient files and will almost always be overridden. Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record); Record.clear(); + } // Header search paths. Record.clear(); diff --git a/clang/test/Modules/diagnostic-options-mismatch.c b/clang/test/Modules/diagnostic-options-mismatch.c new file mode 100644 index 000..cdd6f897729a9d8 --- /dev/null +++ b/clang/test/Modules/diagnostic-options-mismatch.c @@ -0,0 +1,41 @@ +// RUN: rm -rf %t && mkdir -p %t +// RUN: split-file %s %t + +//--- module.modulemap +module Mod { header "mod.h" } +//--- mod.h +//--- tu.c +#include "mod.h" + +// Without any extra compiler flags, mismatched diagnostic options trigger recompilation of modules. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -Wnon-modular-include-in-module +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache1 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -Werror=non-modular-include-in-module -Rmodule-build 2>&1 \ +// RUN: | FileCheck %s --check-prefix=DID-REBUILD +// DID-REBUILD: remark: building module 'Mod' + +// When skipping serialization of diagnostic options, mismatches cannot be detected, old PCM file gets reused. +// +// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/module.modulemap -fmodules-cache-path=%t/cache2 -fdisable-module-hash \ +// RUN: -fsyntax-only %t/tu.c -fmodules-skip-diagnostic-options -Wnon-modular-include-in-module +/
[clang] [clang][deps] Skip slow `UNHASHED_CONTROL_BLOCK` records (PR #69975)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/69975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Strip LLVM options (PR #75405)
@@ -0,0 +1,49 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb1.json.template > %t/cdb1.json + +// RUN: clang-scan-deps -compilation-database %t/cdb1.json -format experimental-full > %t/result1.txt +// RUN: FileCheck %s -input-file %t/result1.txt + +// Verify that secondary actions get stripped, and that there's a single version jansvoboda11 wrote: What do you mean by "secondary actions"? https://github.com/llvm/llvm-project/pull/75405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Strip LLVM options (PR #75405)
@@ -0,0 +1,49 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb1.json.template > %t/cdb1.json + +// RUN: clang-scan-deps -compilation-database %t/cdb1.json -format experimental-full > %t/result1.txt +// RUN: FileCheck %s -input-file %t/result1.txt + +// Verify that secondary actions get stripped, and that there's a single version +// of module A. + +// CHECK:"modules": [ +// CHECK-NEXT: { +// CHECK:"command-line": [ +// CHECK-NOT: "-treat-scalable-fixed-error-as-warning" jansvoboda11 wrote: I'd expect this to check for "-mllvm -stackmap-version=2" from the TU invocation, what's the reason for checking "-treat-scalable-fixed-error-as-warning" instead? https://github.com/llvm/llvm-project/pull/75405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Strip LLVM options (PR #75405)
https://github.com/jansvoboda11 approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/75405 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)
jansvoboda11 wrote: > This seems like something that shouldn't be in the compiler binary itself. > There should be a separate binary for this type of thing. Discussed here: https://discourse.llvm.org/t/rfc-modules-build-daemon-build-system-agnostic-support-for-explicitly-built-modules/71524 https://github.com/llvm/llvm-project/pull/67562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)
jansvoboda11 wrote: The new tests started failing on Apple build bot: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/38908/ Can you please fix these or revert the commit? https://github.com/llvm/llvm-project/pull/65815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang-tools-extra] [llvm-objcopy] Add --gap-fill and --pad-to options (PR #65815)
jansvoboda11 wrote: > Thanks for letting me know, I'm preparing a fix. By the way, is there a way > to run a buildbot manually? Thanks! Not that I know of. https://github.com/llvm/llvm-project/pull/65815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [lldb] [clang] Split out DebugOptions.def into its own top-level options group. (PR #75530)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/75530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [llvm] [clang] Split out DebugOptions.def into its own top-level options group. (PR #75530)
https://github.com/jansvoboda11 requested changes to this pull request. This looks pretty nice, I left just a couple of notes. You might want to take a look at the CI failures. https://github.com/llvm/llvm-project/pull/75530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [lldb] [clang] Split out DebugOptions.def into its own top-level options group. (PR #75530)
@@ -11,46 +11,13 @@ namespace clang { -CodeGenOptions::CodeGenOptions() { -#define CODEGENOPT(Name, Bits, Default) Name = Default; -#define ENUM_CODEGENOPT(Name, Type, Bits, Default) set##Name(Default); -#include "clang/Basic/CodeGenOptions.def" +CodeGenOptions::CodeGenOptions() { resetNonModularOptions(); } jansvoboda11 wrote: I find this confusing: why are we resetting only the non-modular options here? I'd expect all options to be set to default here. Maybe create extra helper function `resetAllOptions()` that will call to `resetNonModularOptions()` and explain how modular options are handled? https://github.com/llvm/llvm-project/pull/75530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] [llvm] [clang] Split out DebugOptions.def into its own top-level options group. (PR #75530)
@@ -11,6 +11,7 @@ #include "clang/APINotes/APINotesOptions.h" #include "clang/Basic/CodeGenOptions.h" +#include "clang/Basic/DebugOptions.h" jansvoboda11 wrote: Wouldn't a forward declaration be enough in this case? https://github.com/llvm/llvm-project/pull/75530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [lldb] [clang] [clang] Split out DebugOptions.def into its own top-level options group. (PR #75530)
@@ -224,19 +233,20 @@ class CompilerInvocation : public CompilerInvocationBase { /// @{ // Note: These need to be pulled in manually. Otherwise, they get hidden by // the mutable getters with the same names. - using CompilerInvocationBase::getLangOpts; - using CompilerInvocationBase::getTargetOpts; - using CompilerInvocationBase::getDiagnosticOpts; - using CompilerInvocationBase::getHeaderSearchOpts; - using CompilerInvocationBase::getPreprocessorOpts; using CompilerInvocationBase::getAnalyzerOpts; - using CompilerInvocationBase::getMigratorOpts; using CompilerInvocationBase::getAPINotesOpts; using CompilerInvocationBase::getCodeGenOpts; + using CompilerInvocationBase::getDebugOpts; jansvoboda11 wrote: Can you undo the reordering here? Let's just add the debug options into the right place and then optionally extract this change into a separate trivially NFC commit (and maybe reorder the member variables to match). https://github.com/llvm/llvm-project/pull/75530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] [llvm] [clang] Split out DebugOptions.def into its own top-level options group. (PR #75530)
@@ -1722,6 +1738,11 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, #include "clang/Driver/Options.inc" #undef CODEGEN_OPTION_WITH_MARSHALLING +#define DEBUG_OPTION_WITH_MARSHALLING(...) \ jansvoboda11 wrote: I'd expect this to go into separate `ParseDebugArgs()` function (which is declared in the header but not defined). https://github.com/llvm/llvm-project/pull/75530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
jansvoboda11 wrote: Ping. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Make sure ninja can clean "ClangdXPC.framework" (PR #75669)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/75669 After building the ClangdXPC target, `ninja clean` fails with the following error: ``` ninja: error: remove(lib/ClangdXPC.framework): Directory not empty ninja: error: remove(/lib/ClangdXPC.framework): Directory not empty ``` I did not find better way to make this work. I guess we could list all generated files (and directories) in `OUTPUT` of the custom command, but that seems fairly tedious/fragile. >From f1785d70c9fb9eb9d23f7aa365f3f6993ad7206c Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Fri, 15 Dec 2023 15:07:00 -0800 Subject: [PATCH] [clangd] Make sure ninja can clean "ClangdXPC.framework" After building the ClangdXPC target, `ninja clean` fails with the following error: ``` ninja: error: remove(lib/ClangdXPC.framework): Directory not empty ninja: error: remove(/lib/ClangdXPC.framework): Directory not empty ``` I did not find better way to make this work. I guess we could list all generated files (and directories) in `OUTPUT` of the custom command, but that seems fairly tedious/fragile. --- .../clangd/xpc/cmake/modules/CreateClangdXPCFramework.cmake | 6 ++ 1 file changed, 6 insertions(+) diff --git a/clang-tools-extra/clangd/xpc/cmake/modules/CreateClangdXPCFramework.cmake b/clang-tools-extra/clangd/xpc/cmake/modules/CreateClangdXPCFramework.cmake index 46738a204ace14..d5ba44962dd528 100644 --- a/clang-tools-extra/clangd/xpc/cmake/modules/CreateClangdXPCFramework.cmake +++ b/clang-tools-extra/clangd/xpc/cmake/modules/CreateClangdXPCFramework.cmake @@ -71,6 +71,12 @@ macro(create_clangd_xpc_framework target name) ${CLANGD_FRAMEWORK_LOCATION} ) + set_property( +TARGET ClangdXPC +APPEND +PROPERTY ADDITIONAL_CLEAN_FILES ${CLANGD_FRAMEWORK_LOCATION} + ) + # clangd is already signed as a standalone executable, so it must be forced. llvm_codesign(ClangdXPC BUNDLE_PATH "${CLANGD_FRAMEWORK_OUT_LOCATION}/XPCServices/${CLANGD_XPC_SERVICE_NAME}.xpc/" FORCE) # ClangdXPC library is already signed as a standalone library, so it must be forced. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][nfc] Add `-fdiagnostics-show-profile-count` option to show real loop count from instr-profile (PR #75021)
@@ -2091,6 +2091,12 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, bool UsingProfile = UsingSampleProfile || !Opts.ProfileInstrumentUsePath.empty(); + if (Args.hasArg(options::OPT_fdiagnostics_show_profile_count) && jansvoboda11 wrote: If this option exists in `-cc1` only to emit a warning without being used for anything else, I suggest moving the diagnostic to the driver and removing the `-cc1` option. https://github.com/llvm/llvm-project/pull/75021 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)
jansvoboda11 wrote: We set `RedirectingFileSystem::HasBeenUsed` to `true` while implicitly compiling one module, but then hand off the same VFS object to implicit compile of another module. This will cause all modules discovered later to incorrectly inherit that bit from previous modules. This will also introduce non-determinism based on the TU that reached a module first (and which modules it imported on the way). Here's a test case: ``` // This test checks that VFS usage doesn't leak between modules. // RUN: rm -rf %t && split-file %s %t // RUN: sed -e "s|DIR|%/t|g" %t/build/cdb.json.in > %t/build/cdb.json // RUN: sed -e "s|DIR|%/t|g" %t/build/vfs.yaml.in > %t/build/vfs.yaml // RUN: clang-scan-deps -compilation-database %t/build/cdb.json \ // RUN: -format experimental-full --optimize-args=vfs > %t/deps.json // RUN: cat %t/deps.json | sed 's:\?:/:g' | FileCheck %s -DPREFIX=%/t // CHECK: { // CHECK-NEXT: "modules": [ // CHECK-NEXT: { // CHECK-NEXT: "clang-module-deps": [ // CHECK-NEXT: { // CHECK-NEXT: "context-hash": "{{.*}}", // CHECK-NEXT: "module-name": "B" // CHECK-NEXT: }, // CHECK-NEXT: { // CHECK-NEXT: "context-hash": "{{.*}}", // CHECK-NEXT: "module-name": "C" // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/moduleA/module.modulemap", // CHECK-NEXT: "command-line": [ // Module A needs the VFS overlay because its dependency, module B, needs it. // CHECK: "-ivfsoverlay" // CHECK-NEXT: "[[PREFIX]]/build/vfs.yaml" // CHECK:], // CHECK-NEXT: "context-hash": "{{.*}}", // CHECK-NEXT: "file-deps": [ // CHECK:], // CHECK-NEXT: "name": "A" // CHECK-NEXT: }, // CHECK-NEXT: { // CHECK-NEXT: "clang-module-deps": [], // CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/moduleB/module.modulemap", // CHECK-NEXT: "command-line": [ // Module B needs the VFS overlay because it provides the header referred to by the module map. // CHECK: "-ivfsoverlay" // CHECK-NEXT: "[[PREFIX]]/build/vfs.yaml" // CHECK:], // CHECK-NEXT: "context-hash": "{{.*}}", // CHECK-NEXT: "file-deps": [ // CHECK:], // CHECK-NEXT: "name": "B" // CHECK-NEXT: }, // CHECK-NEXT: { // CHECK-NEXT: "clang-module-deps": [], // CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/moduleC/module.modulemap", // CHECK-NEXT: "command-line": [ // Module C doesn't need the VFS overlay. // CHECK-NOT: "-ivfsoverlay" // CHECK:], // CHECK-NEXT: "context-hash": "{{.*}}", // CHECK-NEXT: "file-deps": [ // CHECK:], // CHECK-NEXT: "name": "C" // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "translation-units": [ // CHECK:] // CHECK: } //--- build/cdb.json.in [{ "directory": "DIR", "command": "clang -c DIR/tu.m -I DIR/moduleA -I DIR/moduleB -I DIR/moduleC -fmodules -fmodules-cache-path=DIR/cache -fimplicit-module-maps -ivfsoverlay DIR/build/vfs.yaml", "file": "DIR/tu.m" }] //--- build/vfs.yaml.in { "version": 0, "case-sensitive": "false", "roots": [ { "contents": [ { "external-contents": "DIR/build/B.h", "name": "B.h", "type": "file" } ], "name": "DIR/moduleB", "type": "directory" } ] } //--- tu.m @import A; //--- moduleA/module.modulemap module A { header "A.h" } //--- moduleA/A.h @import B; @import C; //--- moduleB/module.modulemap module B { header "B.h" } //--- build/B.h //--- moduleC/module.modulemap module C { header "C.h" } //--- moduleC/C.h ``` https://github.com/llvm/llvm-project/pull/73734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][DependencyScanner] Remove unused -ivfsoverlay files (PR #73734)
jansvoboda11 wrote: I'm not thrilled by the chosen implementation strategy. Disabling tracking during parts of header search does not sound obviously correct to me. Module map search can have subtle side-effects, and I wouldn't be suprised if this feature was affected by one. That said, I haven't been able to come up with a case where this would yield incorrect results. An alternative implementation I'd be more confident in would be to grab the set of files we know affect the compilation (built on top of `ASTWriter::collectNonAffectingInputFiles()`) and call `FileManager::getOptionalFileRef()` with their `FileEntryRef::getNameAsRequested()`. https://github.com/llvm/llvm-project/pull/73734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Reset codegen options. (PR #74006)
@@ -4770,9 +4770,20 @@ std::string CompilerInvocation::getModuleHash() const { // When compiling with -gmodules, also hash -fdebug-prefix-map as it // affects the debug info in the PCM. - if (getCodeGenOpts().DebugTypeExtRefs) + if (getHeaderSearchOpts().ModuleFormat == "obj") { jansvoboda11 wrote: Why did this change? https://github.com/llvm/llvm-project/pull/74006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Reset codegen options. (PR #74006)
@@ -515,6 +430,8 @@ ENUM_CODEGENOPT(ZeroCallUsedRegs, llvm::ZeroCallUsedRegs::ZeroCallUsedRegsKind, /// non-deleting destructors. (No effect on Microsoft ABI.) CODEGENOPT(CtorDtorReturnThis, 1, 0) +#include "DebugOptions.def" jansvoboda11 wrote: Do you think it would make sense to eventually treat these as separate top-level `.def` files? https://github.com/llvm/llvm-project/pull/74006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Reset codegen options. (PR #74006)
@@ -515,6 +430,8 @@ ENUM_CODEGENOPT(ZeroCallUsedRegs, llvm::ZeroCallUsedRegs::ZeroCallUsedRegsKind, /// non-deleting destructors. (No effect on Microsoft ABI.) CODEGENOPT(CtorDtorReturnThis, 1, 0) +#include "DebugOptions.def" jansvoboda11 wrote: Got it. In that case I suggest leaving a FIXME behind. https://github.com/llvm/llvm-project/pull/74006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Reset codegen options. (PR #74006)
@@ -4770,9 +4770,20 @@ std::string CompilerInvocation::getModuleHash() const { // When compiling with -gmodules, also hash -fdebug-prefix-map as it // affects the debug info in the PCM. - if (getCodeGenOpts().DebugTypeExtRefs) + if (getHeaderSearchOpts().ModuleFormat == "obj") { jansvoboda11 wrote: Could we extract this into a separate patch then? Seems like this change is mostly orthogonal to option pruning and could be done in a prep patch. https://github.com/llvm/llvm-project/pull/74006 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [clang][tidy] Ensure rewriter has the correct CWD (PR #67839)
jansvoboda11 wrote: Ping @PiotrZSL. https://github.com/llvm/llvm-project/pull/67839 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Use the exports file on all Unix systems (PR #76742)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/76742 None >From 9e6eb5fbf9e1b540c976d7102dd090622734fd40 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 2 Jan 2024 10:44:45 -0800 Subject: [PATCH] [libclang] Use the exports file on all Unix systems --- clang/tools/libclang/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/tools/libclang/CMakeLists.txt b/clang/tools/libclang/CMakeLists.txt index 4f23065a247274..abc7f0eeb552a0 100644 --- a/clang/tools/libclang/CMakeLists.txt +++ b/clang/tools/libclang/CMakeLists.txt @@ -98,7 +98,6 @@ if(MSVC) endif() if (UNIX AND NOT APPLE) - set(LLVM_EXPORTED_SYMBOL_FILE) set(USE_VERSION_SCRIPT ${LLVM_HAVE_LINK_VERSION_SCRIPT}) endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
jansvoboda11 wrote: Thanks for reporting @jrmwng, I'll look into it. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
jansvoboda11 wrote: > Thanks for reporting @jrmwng, I'll look into it. I wasn't able to reproduce locally on Windows 10. Can you please share detailed reproduction steps and any other config that may be relevant? It seems your realpath to the LLVM repo is `"C:\GitHub\llvm-project"` and the `check-all` target is built using `"c:\github\llvm-project"`. This works on my machine, so something else must be at play. https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix test for case-insensitive absolute includes (PR #76985)
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/76985 When the test was ran via a symlink path, the diagnostic did not fire, since the `%t` path in the include directive would differ in more than just a case from the real path of the header file. >From c8ebe899978ade58fa014a3c71a7b5888be865bd Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 4 Jan 2024 08:57:40 -0800 Subject: [PATCH] [clang] Fix test for case-insensitive absolute includes When the test was ran via a symlink path (e.g. `ninja -C /symlink/llvm-project/build check-clang`), the diagnostic would not fire, since `%t` path in the include directive would differ in more than just a case from the real path of the header file. --- clang/test/Lexer/case-insensitive-include-absolute.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Lexer/case-insensitive-include-absolute.c b/clang/test/Lexer/case-insensitive-include-absolute.c index 6247e4808c7fa2..ef9e131c45df59 100644 --- a/clang/test/Lexer/case-insensitive-include-absolute.c +++ b/clang/test/Lexer/case-insensitive-include-absolute.c @@ -1,8 +1,8 @@ // REQUIRES: case-insensitive-filesystem // RUN: rm -rf %t && split-file %s %t -// RUN: sed "s|DIR|%/t|g" %t/tu.c.in > %t/tu.c -// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s --DDIR=%/t +// RUN: sed "s|DIR|%{/t:real}|g" %t/tu.c.in > %t/tu.c +// RUN: %clang_cc1 -fsyntax-only %t/tu.c 2>&1 | FileCheck %s -DDIR=%{/t:real} //--- header.h //--- tu.c.in ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
jansvoboda11 wrote: Thank you. I think the symlink is most likely the culprit. I'll be able to confirm on my Windows machine tomorrow. For now, I have a speculative fix: https://github.com/llvm/llvm-project/pull/76985 https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)
@@ -0,0 +1,60 @@ +//=== SocketMsgSupport.cpp ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/raw_socket_stream.h" + +#include +#include + +using namespace llvm; + +namespace clang::tooling::cc1modbuildd { + +Expected> +connectToSocket(StringRef SocketPath) { + + Expected> MaybeClient = + raw_socket_stream::createConnectedUnix(SocketPath); + if (!MaybeClient) +return std::move(MaybeClient.takeError()); + + return std::move(*MaybeClient); +} + +Expected readBufferFromSocket(raw_socket_stream &Socket) { + + constexpr const unsigned short MAX_BUFFER = 4096; + char Buffer[MAX_BUFFER]; + std::string ReturnBuffer; + + ssize_t n = 0; + while ((n = Socket.read(Buffer, MAX_BUFFER)) > 0) { +ReturnBuffer.append(Buffer, n); +// Read until \n... encountered which is the last line of a YAML document +if (ReturnBuffer.find("\n...") != std::string::npos) jansvoboda11 wrote: IIUC `"\n..."` is always going to be at the end of `ReturnBuffer`, right? Could we replace the call to `std::string::find()` with `StringRef(ReturnBuffer).ends_with("\n...")`? Considering `"\n..."` constant, I think this would reduce the complexity of the `while` loop from `O(n*n)` to `O(n)`. https://github.com/llvm/llvm-project/pull/67562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)
@@ -0,0 +1,48 @@ +//===-- Utils.h ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Functions required by both the frontend and the module build daemon +// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_MODULEBUILDDAEMON_UTILS_H +#define LLVM_CLANG_TOOLING_MODULEBUILDDAEMON_UTILS_H + +#include "llvm/Support/Error.h" + +#include + +#ifdef _WIN32 +#include +#else +#include +#endif + +namespace clang::tooling::cc1modbuildd { + +constexpr const std::string_view SOCKET_FILE_NAME = "mbd.sock"; jansvoboda11 wrote: You should be able to drop `const` here, since that's implied by `constexpr`. https://github.com/llvm/llvm-project/pull/67562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)
@@ -383,6 +383,9 @@ class FrontendOptions { LLVM_PREFERRED_TYPE(bool) unsigned ModulesShareFileManager : 1; + /// Connect to module build daemon + unsigned ModuleBuildDaemon : 1; jansvoboda11 wrote: Mark this with `LLVM_PREFERRED_TYPE(bool)`, like the other bool-like bitfield in this class. https://github.com/llvm/llvm-project/pull/67562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang][MBD] set up module build daemon infrastructure (PR #67562)
@@ -0,0 +1,48 @@ +//===-- Utils.h ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Functions required by both the frontend and the module build daemon +// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_MODULEBUILDDAEMON_UTILS_H +#define LLVM_CLANG_TOOLING_MODULEBUILDDAEMON_UTILS_H + +#include "llvm/Support/Error.h" + +#include + +#ifdef _WIN32 +#include +#else +#include +#endif + +namespace clang::tooling::cc1modbuildd { + +constexpr const std::string_view SOCKET_FILE_NAME = "mbd.sock"; +constexpr const std::string_view STDOUT_FILE_NAME = "mbd.out"; +constexpr const std::string_view STDERR_FILE_NAME = "mbd.err"; +constexpr const std::string_view MODULE_BUILD_DAEMON_FLAG = "-cc1modbuildd"; + +// A llvm::raw_socket_stream uses sockaddr_un +constexpr const size_t SOCKET_ADDR_MAX_LENGTH = sizeof(sockaddr_un::sun_path); + +constexpr const size_t BASEPATH_MAX_LENGTH = +SOCKET_ADDR_MAX_LENGTH - std::string_view(SOCKET_FILE_NAME).length(); jansvoboda11 wrote: Is constructing new `std::string_view` from `SOCKET_FILE_NAME` necessary? https://github.com/llvm/llvm-project/pull/67562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libclang] Use the exports file on all Unix systems (PR #76742)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/76742 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix test for case-insensitive absolute includes (PR #76985)
https://github.com/jansvoboda11 edited https://github.com/llvm/llvm-project/pull/76985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix test for case-insensitive absolute includes (PR #76985)
https://github.com/jansvoboda11 ready_for_review https://github.com/llvm/llvm-project/pull/76985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Fix test for case-insensitive absolute includes (PR #76985)
https://github.com/jansvoboda11 closed https://github.com/llvm/llvm-project/pull/76985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][lex] Fix non-portability diagnostics with absolute path (PR #74782)
jansvoboda11 wrote: @jrmwng PR #76985 (landed in 853b133) seems to have resolved this issue for me. Can you confirm that on your end? https://github.com/llvm/llvm-project/pull/74782 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][modules] Remove `_Private` suffix from framework auto-link hints. (PR #77120)
@@ -0,0 +1,25 @@ +// Test that autolink hints for frameworks don't use the private module name. jansvoboda11 wrote: Why is this desired? What happens if the private module name is provided? https://github.com/llvm/llvm-project/pull/77120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits