[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function (PR #88447)
https://github.com/akyrtzi closed https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function (PR #88447)
akyrtzi wrote: > I assume you'll call resetBenignCodeGenOptions() from > ModuleDepCollector::applyDiscoveredDependencies() in a follow-up, non-NFC > patch, right? @jansvoboda11 see updated commit, I made the change per your suggestion. https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function (PR #88447)
https://github.com/akyrtzi edited https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function (PR #88447)
https://github.com/akyrtzi edited https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/88447 >From 8b21183d5e836e0900ce051dab91f0a399ed83e5 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Thu, 11 Apr 2024 14:57:40 -0700 Subject: [PATCH] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function The new function is about clearing out benign codegen options and can be applied for PCH invocations as well. --- .../DependencyScanning/ModuleDepCollector.h | 5 +++ .../DependencyScanning/ModuleDepCollector.cpp | 36 --- clang/test/ClangScanDeps/removed-args.c | 28 +++ 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index 081899cc2c8503..da51292296a90f 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -308,6 +308,11 @@ class ModuleDepCollector final : public DependencyCollector { ModuleDeps ); }; +/// Resets codegen options that don't affect modules/PCH. +void resetBenignCodeGenOptions(frontend::ActionKind ProgramAction, + const LangOptions , + CodeGenOptions ); + } // end namespace dependencies } // end namespace tooling } // end namespace clang diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 94ccbd3351b09d..e19f19b2528c15 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -154,6 +154,26 @@ void ModuleDepCollector::addOutputPaths(CowCompilerInvocation , } } +void dependencies::resetBenignCodeGenOptions(frontend::ActionKind ProgramAction, + const LangOptions , + CodeGenOptions ) { + // TODO: Figure out better way to set options to their default value. + if (ProgramAction == frontend::GenerateModule) { +CGOpts.MainFileName.clear(); +CGOpts.DwarfDebugFlags.clear(); + } + if (ProgramAction == frontend::GeneratePCH || + (ProgramAction == frontend::GenerateModule && !LangOpts.ModulesCodegen)) { +CGOpts.DebugCompilationDir.clear(); +CGOpts.CoverageCompilationDir.clear(); +CGOpts.CoverageDataFile.clear(); +CGOpts.CoverageNotesFile.clear(); +CGOpts.ProfileInstrumentUsePath.clear(); +CGOpts.SampleProfileFile.clear(); +CGOpts.ProfileRemappingFile.clear(); + } +} + static CowCompilerInvocation makeCommonInvocationForModuleBuild(CompilerInvocation CI) { CI.resetNonModularOptions(); @@ -167,18 +187,8 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) { // LLVM options are not going to affect the AST CI.getFrontendOpts().LLVMArgs.clear(); - // TODO: Figure out better way to set options to their default value. - CI.getCodeGenOpts().MainFileName.clear(); - CI.getCodeGenOpts().DwarfDebugFlags.clear(); - if (!CI.getLangOpts().ModulesCodegen) { -CI.getCodeGenOpts().DebugCompilationDir.clear(); -CI.getCodeGenOpts().CoverageCompilationDir.clear(); -CI.getCodeGenOpts().CoverageDataFile.clear(); -CI.getCodeGenOpts().CoverageNotesFile.clear(); -CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); -CI.getCodeGenOpts().SampleProfileFile.clear(); -CI.getCodeGenOpts().ProfileRemappingFile.clear(); - } + resetBenignCodeGenOptions(frontend::GenerateModule, CI.getLangOpts(), +CI.getCodeGenOpts()); // Map output paths that affect behaviour to "-" so their existence is in the // context hash. The final path will be computed in addOutputPaths. @@ -342,6 +352,8 @@ static bool needsModules(FrontendInputFile FIF) { void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation ) { CI.clearImplicitModuleBuildOptions(); + resetBenignCodeGenOptions(CI.getFrontendOpts().ProgramAction, +CI.getLangOpts(), CI.getCodeGenOpts()); if (llvm::any_of(CI.getFrontendOpts().Inputs, needsModules)) { Preprocessor = ScanInstance.getPreprocessor(); diff --git a/clang/test/ClangScanDeps/removed-args.c b/clang/test/ClangScanDeps/removed-args.c index f49e4ead82f7bf..3e108f0549450c 100644 --- a/clang/test/ClangScanDeps/removed-args.c +++ b/clang/test/ClangScanDeps/removed-args.c @@ -93,3 +93,31 @@ // CHECK-NOT: "-fmodules-prune-interval= // CHECK-NOT: "-fmodules-prune-after= // CHECK:], + +// Check for removed args for PCH invocations. + +// RUN: split-file %s %t +// RUN: sed "s|DIR|%/t|g" %t/cdb-pch.json.template > %t/cdb-pch.json +// RUN: clang-scan-deps -compilation-database %t/cdb-pch.json -format
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
@@ -308,6 +308,9 @@ class ModuleDepCollector final : public DependencyCollector { ModuleDeps ); }; +/// Resets some options that introduce dependencies unnecessarily. +void removeUnnecessaryDependencies(CompilerInvocation , bool ForModuleBuild); akyrtzi wrote: New name in updated commit. https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
@@ -154,6 +154,20 @@ void ModuleDepCollector::addOutputPaths(CowCompilerInvocation , } } +void dependencies::removeUnnecessaryDependencies(CompilerInvocation , + bool ForModuleBuild) { + if (CI.getFrontendOpts().ProgramAction == frontend::GeneratePCH || + (ForModuleBuild && !CI.getLangOpts().ModulesCodegen)) { +CI.getCodeGenOpts().DebugCompilationDir.clear(); +CI.getCodeGenOpts().CoverageCompilationDir.clear(); +CI.getCodeGenOpts().CoverageDataFile.clear(); +CI.getCodeGenOpts().CoverageNotesFile.clear(); +CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); +CI.getCodeGenOpts().SampleProfileFile.clear(); +CI.getCodeGenOpts().ProfileRemappingFile.clear(); akyrtzi wrote: See updated commit. https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
https://github.com/akyrtzi edited https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/88447 >From f96eadb5d41d2433cf0b114556eaaa5f0a66e250 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Thu, 11 Apr 2024 14:57:40 -0700 Subject: [PATCH] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC The new function is about clearing out benign codegen options and is a bit more general. --- .../DependencyScanning/ModuleDepCollector.h | 5 +++ .../DependencyScanning/ModuleDepCollector.cpp | 34 --- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index 081899cc2c8503..da51292296a90f 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -308,6 +308,11 @@ class ModuleDepCollector final : public DependencyCollector { ModuleDeps ); }; +/// Resets codegen options that don't affect modules/PCH. +void resetBenignCodeGenOptions(frontend::ActionKind ProgramAction, + const LangOptions , + CodeGenOptions ); + } // end namespace dependencies } // end namespace tooling } // end namespace clang diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 94ccbd3351b09d..aceee4ba0f898f 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -154,6 +154,26 @@ void ModuleDepCollector::addOutputPaths(CowCompilerInvocation , } } +void dependencies::resetBenignCodeGenOptions(frontend::ActionKind ProgramAction, + const LangOptions , + CodeGenOptions ) { + // TODO: Figure out better way to set options to their default value. + if (ProgramAction == frontend::GenerateModule) { +CGOpts.MainFileName.clear(); +CGOpts.DwarfDebugFlags.clear(); + } + if (ProgramAction == frontend::GeneratePCH || + (ProgramAction == frontend::GenerateModule && !LangOpts.ModulesCodegen)) { +CGOpts.DebugCompilationDir.clear(); +CGOpts.CoverageCompilationDir.clear(); +CGOpts.CoverageDataFile.clear(); +CGOpts.CoverageNotesFile.clear(); +CGOpts.ProfileInstrumentUsePath.clear(); +CGOpts.SampleProfileFile.clear(); +CGOpts.ProfileRemappingFile.clear(); + } +} + static CowCompilerInvocation makeCommonInvocationForModuleBuild(CompilerInvocation CI) { CI.resetNonModularOptions(); @@ -167,18 +187,8 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) { // LLVM options are not going to affect the AST CI.getFrontendOpts().LLVMArgs.clear(); - // TODO: Figure out better way to set options to their default value. - CI.getCodeGenOpts().MainFileName.clear(); - CI.getCodeGenOpts().DwarfDebugFlags.clear(); - if (!CI.getLangOpts().ModulesCodegen) { -CI.getCodeGenOpts().DebugCompilationDir.clear(); -CI.getCodeGenOpts().CoverageCompilationDir.clear(); -CI.getCodeGenOpts().CoverageDataFile.clear(); -CI.getCodeGenOpts().CoverageNotesFile.clear(); -CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); -CI.getCodeGenOpts().SampleProfileFile.clear(); -CI.getCodeGenOpts().ProfileRemappingFile.clear(); - } + resetBenignCodeGenOptions(frontend::GenerateModule, CI.getLangOpts(), +CI.getCodeGenOpts()); // Map output paths that affect behaviour to "-" so their existence is in the // context hash. The final path will be computed in addOutputPaths. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
@@ -154,6 +154,20 @@ void ModuleDepCollector::addOutputPaths(CowCompilerInvocation , } } +void dependencies::removeUnnecessaryDependencies(CompilerInvocation , + bool ForModuleBuild) { + if (CI.getFrontendOpts().ProgramAction == frontend::GeneratePCH || + (ForModuleBuild && !CI.getLangOpts().ModulesCodegen)) { +CI.getCodeGenOpts().DebugCompilationDir.clear(); +CI.getCodeGenOpts().CoverageCompilationDir.clear(); +CI.getCodeGenOpts().CoverageDataFile.clear(); +CI.getCodeGenOpts().CoverageNotesFile.clear(); +CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); +CI.getCodeGenOpts().SampleProfileFile.clear(); +CI.getCodeGenOpts().ProfileRemappingFile.clear(); akyrtzi wrote: I had an offline discussion with Jan about how to better factor-out this piece of code. https://github.com/llvm/llvm-project/pull/88447 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC (PR #88447)
https://github.com/akyrtzi created https://github.com/llvm/llvm-project/pull/88447 The function is named `removeUnnecessaryDependencies` and is a bit more general that could be used from other places as well. >From 0344d1890028b178d54d69fba239149b7d96fa30 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Thu, 11 Apr 2024 14:57:40 -0700 Subject: [PATCH] [clang/DependencyScanning/ModuleDepCollector] Refactor part of `makeCommonInvocationForModuleBuild` into its own function, NFC The function is named `removeUnnecessaryDependencies` and is a bit more general that could be used from other places as well. --- .../DependencyScanning/ModuleDepCollector.h | 3 +++ .../DependencyScanning/ModuleDepCollector.cpp | 25 --- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index 081899cc2c8503..b971269c4983ed 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -308,6 +308,9 @@ class ModuleDepCollector final : public DependencyCollector { ModuleDeps ); }; +/// Resets some options that introduce dependencies unnecessarily. +void removeUnnecessaryDependencies(CompilerInvocation , bool ForModuleBuild); + } // end namespace dependencies } // end namespace tooling } // end namespace clang diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 94ccbd3351b09d..fd425ff7c4cafe 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -154,6 +154,20 @@ void ModuleDepCollector::addOutputPaths(CowCompilerInvocation , } } +void dependencies::removeUnnecessaryDependencies(CompilerInvocation , + bool ForModuleBuild) { + if (CI.getFrontendOpts().ProgramAction == frontend::GeneratePCH || + (ForModuleBuild && !CI.getLangOpts().ModulesCodegen)) { +CI.getCodeGenOpts().DebugCompilationDir.clear(); +CI.getCodeGenOpts().CoverageCompilationDir.clear(); +CI.getCodeGenOpts().CoverageDataFile.clear(); +CI.getCodeGenOpts().CoverageNotesFile.clear(); +CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); +CI.getCodeGenOpts().SampleProfileFile.clear(); +CI.getCodeGenOpts().ProfileRemappingFile.clear(); + } +} + static CowCompilerInvocation makeCommonInvocationForModuleBuild(CompilerInvocation CI) { CI.resetNonModularOptions(); @@ -170,15 +184,8 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) { // TODO: Figure out better way to set options to their default value. CI.getCodeGenOpts().MainFileName.clear(); CI.getCodeGenOpts().DwarfDebugFlags.clear(); - if (!CI.getLangOpts().ModulesCodegen) { -CI.getCodeGenOpts().DebugCompilationDir.clear(); -CI.getCodeGenOpts().CoverageCompilationDir.clear(); -CI.getCodeGenOpts().CoverageDataFile.clear(); -CI.getCodeGenOpts().CoverageNotesFile.clear(); -CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); -CI.getCodeGenOpts().SampleProfileFile.clear(); -CI.getCodeGenOpts().ProfileRemappingFile.clear(); - } + + removeUnnecessaryDependencies(CI, /*ForModuleBuild=*/true); // Map output paths that affect behaviour to "-" so their existence is in the // context hash. The final path will be computed in addOutputPaths. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Remove pgo profile flags from modules (PR #87724)
https://github.com/akyrtzi closed https://github.com/llvm/llvm-project/pull/87724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Remove pgo profile flags from modules (PR #87724)
https://github.com/akyrtzi created https://github.com/llvm/llvm-project/pull/87724 These are not necessary when not performing codegen. >From 4a2b299e264ce2833786ab035cbb0938284c73ac Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Thu, 4 Apr 2024 15:57:28 -0700 Subject: [PATCH] [clang][deps] Remove pgo profile flags from modules These are not necessary when not performing codegen. --- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp | 3 +++ .../test/ClangScanDeps/Inputs/removed-args/cdb.json.template | 2 +- clang/test/ClangScanDeps/removed-args.c | 4 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index eb5c50c35428fe..94ccbd3351b09d 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -175,6 +175,9 @@ makeCommonInvocationForModuleBuild(CompilerInvocation CI) { CI.getCodeGenOpts().CoverageCompilationDir.clear(); CI.getCodeGenOpts().CoverageDataFile.clear(); CI.getCodeGenOpts().CoverageNotesFile.clear(); +CI.getCodeGenOpts().ProfileInstrumentUsePath.clear(); +CI.getCodeGenOpts().SampleProfileFile.clear(); +CI.getCodeGenOpts().ProfileRemappingFile.clear(); } // Map output paths that affect behaviour to "-" so their existence is in the diff --git a/clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template b/clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template index 7ae3c88aedd8d2..ca56799cd08eab 100644 --- a/clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template +++ b/clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template @@ -1,7 +1,7 @@ [ { "directory": "DIR", -"command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-validate-once-per-build-session -fbuild-session-file=DIR/build-session -fmodules-prune-interval=123 -fmodules-prune-after=123 -fmodules-cache-path=DIR/cache -include DIR/header.h -grecord-command-line -fdebug-compilation-dir=DIR/debug -fcoverage-compilation-dir=DIR/coverage -ftest-coverage -o DIR/tu.o -serialize-diagnostics DIR/tu.diag -MT tu -MD -MF DIR/tu.d", +"command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-validate-once-per-build-session -fbuild-session-file=DIR/build-session -fmodules-prune-interval=123 -fmodules-prune-after=123 -fmodules-cache-path=DIR/cache -include DIR/header.h -grecord-command-line -fdebug-compilation-dir=DIR/debug -fcoverage-compilation-dir=DIR/coverage -ftest-coverage -fprofile-instr-use=DIR/tu.profdata -o DIR/tu.o -serialize-diagnostics DIR/tu.diag -MT tu -MD -MF DIR/tu.d", "file": "DIR/tu.c" } ] diff --git a/clang/test/ClangScanDeps/removed-args.c b/clang/test/ClangScanDeps/removed-args.c index 9a4ef25838e465..f49e4ead82f7bf 100644 --- a/clang/test/ClangScanDeps/removed-args.c +++ b/clang/test/ClangScanDeps/removed-args.c @@ -9,6 +9,8 @@ // RUN: rm -rf %t && mkdir %t // RUN: cp %S/Inputs/removed-args/* %t // RUN: touch %t/build-session +// RUN: touch %t/tu.proftext +// RUN: llvm-profdata merge %t/tu.proftext -o %t/tu.profdata // RUN: sed "s|DIR|%/t|g" %S/Inputs/removed-args/cdb.json.template > %t/cdb.json // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json @@ -25,6 +27,7 @@ // CHECK-NOT: "-fcoverage-compilation-dir=" // CHECK-NOT: "-coverage-notes-file // CHECK-NOT: "-coverage-data-file +// CHECK-NOT: "-fprofile-instrument-use-path // CHECK-NOT: "-dwarf-debug-flags" // CHECK-NOT: "-main-file-name" // CHECK-NOT: "-include" @@ -50,6 +53,7 @@ // CHECK-NOT: "-fcoverage-compilation-dir= // CHECK-NOT: "-coverage-notes-file // CHECK-NOT: "-coverage-data-file +// CHECK-NOT: "-fprofile-instrument-use-path // CHECK-NOT: "-dwarf-debug-flags" // CHECK-NOT: "-main-file-name" // CHECK-NOT: "-include" ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][deps] Lazy dependency directives (PR #86347)
https://github.com/akyrtzi approved this pull request. Nice! Note that you implemented rdar://107663951 https://github.com/llvm/llvm-project/pull/86347 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Warn unused functions for FMV based on the target attribute (PR #81302)
https://github.com/akyrtzi approved this pull request. https://github.com/llvm/llvm-project/pull/81302 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][sema] Fix -Wunused-function on target_version'd file-scope Fn's (PR #81167)
https://github.com/akyrtzi approved this pull request. https://github.com/llvm/llvm-project/pull/81167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi closed https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/66122 >From 26b0440d81f4bbf8e666c1c11e200963fa2cddb4 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 79 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 + 3 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..dbe219b6dd8d723 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + llvm::ErrorOr WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..3e53c8fc5740875 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards]; } @@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID( const CachedFileSystemEntry * DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename( StringRef Filename) const { +
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { akyrtzi wrote: Windows failed: ``` Failed Tests (2): Clang-Unit :: Tooling/./ToolingTests.exe/DependencyScanner/ScanDepsWithFS Clang-Unit :: Tooling/./ToolingTests.exe/DependencyScanner/ScanDepsWithModuleLookup ``` I'm going to revert back to `_gnu` variant. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { akyrtzi wrote: I've changed to `is_absolute`, if the the tests pass on Windows we should be good to go. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi resolved https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi edited https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,63 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else if (!WorkingDirForCacheLookup) { +return WorkingDirForCacheLookup.getError(); + } else { +StringRef RelFilename = OriginalFilename; +if (RelFilename.startswith("./")) + RelFilename = RelFilename.substr(strlen("./")); akyrtzi wrote: Done https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/66122 >From 26b0440d81f4bbf8e666c1c11e200963fa2cddb4 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH 1/2] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 79 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 + 3 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..dbe219b6dd8d723 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + llvm::ErrorOr WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..3e53c8fc5740875 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards]; } @@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID( const CachedFileSystemEntry * DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename( StringRef Filename) const { +
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi resolved https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi resolved https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +359,24 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + std::error_code EC = ProxyFileSystem::setCurrentWorkingDirectory(Path); + updateWorkingDirForCacheLookup(); + return EC; +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup() { + llvm::ErrorOr CWD = + getUnderlyingFS().getCurrentWorkingDirectory(); + if (!CWD) { +WorkingDirForCacheLookup = CWD.getError(); + } else if (!llvm::sys::path::is_absolute_gnu(*CWD)) { +WorkingDirForCacheLookup = llvm::errc::argument_out_of_domain; akyrtzi wrote: Changed it to `invalid_argument`. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/66122 >From 1423e870d3bd0acc9554e4ca2b8884d520038844 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 80 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 + 3 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..dbe219b6dd8d723 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + llvm::ErrorOr WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..b11a611de97e192 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards]; } @@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID( const CachedFileSystemEntry * DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename( StringRef Filename) const { +
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + updateWorkingDirForCacheLookup(Path.str()); + return ProxyFileSystem::setCurrentWorkingDirectory(Path); +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup( +llvm::ErrorOr CWD) { + if (CWD && !CWD->empty()) { +WorkingDirForCacheLookup = *CWD; + } else { +// The cache lookup functions will not accept relative paths for safety, so +// at least make it absolute from a "root". +WorkingDirForCacheLookup = "/"; akyrtzi wrote: See latest commit, I've changed it so it became `llvm::ErrorOr WorkingDirForCacheLookup` and `DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry()` errors out if it needs to use `WorkingDirForCacheLookup` and it's erroneous. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/66122 >From 5794986079f3eb0f52dd6089d50d994b4559ed06 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 80 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 + 3 files changed, 118 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..dbe219b6dd8d723 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + llvm::ErrorOr WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..e2c404206170ee9 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards]; } @@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID( const CachedFileSystemEntry * DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename( StringRef Filename) const { +
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + updateWorkingDirForCacheLookup(Path.str()); + return ProxyFileSystem::setCurrentWorkingDirectory(Path); +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup( +llvm::ErrorOr CWD) { + if (CWD && !CWD->empty()) { +WorkingDirForCacheLookup = *CWD; + } else { +// The cache lookup functions will not accept relative paths for safety, so +// at least make it absolute from a "root". +WorkingDirForCacheLookup = "/"; akyrtzi wrote: Oh wait, I see why it is poisoning, I could change it so that it avoids cache lookups if an error occurred? https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + updateWorkingDirForCacheLookup(Path.str()); + return ProxyFileSystem::setCurrentWorkingDirectory(Path); +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup( +llvm::ErrorOr CWD) { + if (CWD && !CWD->empty()) { +WorkingDirForCacheLookup = *CWD; + } else { +// The cache lookup functions will not accept relative paths for safety, so +// at least make it absolute from a "root". +WorkingDirForCacheLookup = "/"; akyrtzi wrote: How is it poisoning the cache? The key for the cache lookup will be "/include/foo.h" and the associated value will be whatever "/include/foo.h" resolves to. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + updateWorkingDirForCacheLookup(Path.str()); + return ProxyFileSystem::setCurrentWorkingDirectory(Path); +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup( +llvm::ErrorOr CWD) { + if (CWD && !CWD->empty()) { +WorkingDirForCacheLookup = *CWD; + } else { +// The cache lookup functions will not accept relative paths for safety, so +// at least make it absolute from a "root". +WorkingDirForCacheLookup = "/"; akyrtzi wrote: > In this case the path will be used when writing to the cache though, right? > That affects scan results for other TUs just like you are fixing in this PR. What is the specific concern? If multiple TUs got errors for working directory and subsequently used the same standard prefix then they may resolve to same path for a relative filename but these TUs are already in an erroneous state already, it's not invalidating their correctness. > I'm not clear what recovery means here My general point is that I don't think we should make the assumption that if a VFS implementation emits an error for `setCurrentWorkingDirectory()` then that VFS instance should not ever get used again subsequently. We could say this is specific for the `DependencyScanningWorkerFilesystem` implementation but I don't see a motivating reason that it should not be able to continue operating instead of crashing if it gets used again. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + updateWorkingDirForCacheLookup(Path.str()); + return ProxyFileSystem::setCurrentWorkingDirectory(Path); +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup( +llvm::ErrorOr CWD) { + if (CWD && !CWD->empty()) { +WorkingDirForCacheLookup = *CWD; + } else { +// The cache lookup functions will not accept relative paths for safety, so +// at least make it absolute from a "root". +WorkingDirForCacheLookup = "/"; akyrtzi wrote: > For the setCurrentWorkingDirectory() call, I think it'd be reasonable to > clear our CWD if the new one is empty or relative, return an error code and > assume the VFS doesn't get used in that state. If it does, hitting one of the > "is file path absolute" assertions is fair game. This would be fine for a command-line compiler invocation where you can emit an error and then "abort", but this is undesirable in the context of a libclang client, like an IDE, where some recovery in the presence of errors is useful; for example, clang could still try to create an AST even in the presence of such kind of errors in order to support the editor, instead of providing no functionality. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else { +PathBuf = WorkingDirForCacheLookup; +llvm::sys::path::append(PathBuf, OriginalFilename); akyrtzi wrote: I made a change to remove leading "./" if the path is relative, I preferred to avoid doing any work if path is absolute, which is the common case. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/66122: >From a457c0cfe749fe00ecde7145e9beb519c254eccd Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 79 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 + 3 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..7c8382024a8a4f4 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + std::string WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..97b6ef764ac621e 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards]; } @@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID( const CachedFileSystemEntry * DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename( StringRef Filename) const { +
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else { +PathBuf = WorkingDirForCacheLookup; +llvm::sys::path::append(PathBuf, OriginalFilename); akyrtzi wrote: Let's say that a filename gets passed in that is like this: "./whatever/./something/t.h" We make it absolute and then call `remove_dots` and it turns into this for the cache lookup: "/cwd/whatever/something/t.h" My intuition is that if a dot exists in the middle of a relative filename, it will likely also be there if clang forms an absolute path using it. So later let's say clang passes this filename to the function: "/cwd/whatever/./something/t.h" Because this is absolute we use it directly for the cache lookup, but this is different than the "dot-less" version so it's a cache miss. My point is if we call `remove_dots` when we absolutize a filename, in order to get a cache hit in a later call where clang passes the associated file as absolute filename, in practice it will defeat the purpose if a dot exists in the middle of the filename (it will be a cache miss), so it may be more profitable (more cache hits) to just remove the `./` prefix. The above is with the assumption that we do nothing to the filename if it is already absolute, are you both suggesting to process the filename and always call `remove_dots` whether the given filename is absolute or not? https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi resolved https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi edited https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else { +PathBuf = WorkingDirForCacheLookup; +llvm::sys::path::append(PathBuf, OriginalFilename); +FilenameForLookup = PathBuf.str(); akyrtzi wrote: Added an assertion. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else { +PathBuf = WorkingDirForCacheLookup; akyrtzi wrote: I've made additional changes, see implementation of `updateWorkingDirForCacheLookup()` where it guarantees it now. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi updated https://github.com/llvm/llvm-project/pull/66122: >From 98e56d4d8d1f138cfc69c035425fafcdd57d5916 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 76 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 ++ 3 files changed, 114 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..7c8382024a8a4f4 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + std::string WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..dd329f41cf5c30a 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards]; } @@ -109,6 +110,7 @@ DependencyScanningFilesystemSharedCache::getShardForUID( const CachedFileSystemEntry * DependencyScanningFilesystemSharedCache::CacheShard::findEntryByFilename( StringRef Filename) const { +
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine ) { return Result.getError(); return DepScanFile::create(Result.get()); } + +std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory( +const Twine ) { + updateWorkingDirForCacheLookup(Path.str()); + return ProxyFileSystem::setCurrentWorkingDirectory(Path); +} + +void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup( +llvm::ErrorOr CWD) { + if (CWD && !CWD->empty()) { +WorkingDirForCacheLookup = *CWD; + } else { +// The cache lookup functions will not accept relative paths for safety, so +// at least make it absolute from a "root". +WorkingDirForCacheLookup = "/"; akyrtzi wrote: This is more of a recovery to avoid assertion hits, assuming the error gets reported. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else { +PathBuf = WorkingDirForCacheLookup; +llvm::sys::path::append(PathBuf, OriginalFilename); akyrtzi wrote: I mean I assume there are various ways that an absolute filename may be formed by clang that contains a dot, and passed to this function. Or is this not possible? https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { akyrtzi wrote: Per doc-comments, `_gnu` variant treats "/" as absolute in both POSIX and Windows (unlike the other variant), which is convenient for ensuring that `WorkingDirForCacheLookup` is always considered absolute (see the `updateWorkingDirForCacheLookup()` implementation). https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
@@ -215,44 +225,57 @@ DependencyScanningWorkerFilesystem::findEntryByFilenameWithWriteThrough( } llvm::ErrorOr -DependencyScanningWorkerFilesystem::computeAndStoreResult(StringRef Filename) { - llvm::ErrorOr Stat = getUnderlyingFS().status(Filename); +DependencyScanningWorkerFilesystem::computeAndStoreResult( +StringRef OriginalFilename, StringRef FilenameForLookup) { + llvm::ErrorOr Stat = + getUnderlyingFS().status(OriginalFilename); if (!Stat) { -if (!shouldCacheStatFailures(Filename)) +if (!shouldCacheStatFailures(OriginalFilename)) return Stat.getError(); const auto = -getOrEmplaceSharedEntryForFilename(Filename, Stat.getError()); -return insertLocalEntryForFilename(Filename, Entry); +getOrEmplaceSharedEntryForFilename(FilenameForLookup, Stat.getError()); +return insertLocalEntryForFilename(FilenameForLookup, Entry); } if (const auto *Entry = findSharedEntryByUID(*Stat)) -return insertLocalEntryForFilename(Filename, *Entry); +return insertLocalEntryForFilename(FilenameForLookup, *Entry); auto TEntry = - Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(Filename); + Stat->isDirectory() ? TentativeEntry(*Stat) : readFile(OriginalFilename); const CachedFileSystemEntry *SharedEntry = [&]() { if (TEntry) { const auto = getOrEmplaceSharedEntryForUID(std::move(*TEntry)); - return (Filename, UIDEntry); + return (FilenameForLookup, UIDEntry); } -return (Filename, TEntry.getError()); +return (FilenameForLookup, + TEntry.getError()); }(); - return insertLocalEntryForFilename(Filename, *SharedEntry); + return insertLocalEntryForFilename(FilenameForLookup, *SharedEntry); } llvm::ErrorOr DependencyScanningWorkerFilesystem::getOrCreateFileSystemEntry( -StringRef Filename, bool DisableDirectivesScanning) { - if (const auto *Entry = findEntryByFilenameWithWriteThrough(Filename)) -return scanForDirectivesIfNecessary(*Entry, Filename, +StringRef OriginalFilename, bool DisableDirectivesScanning) { + StringRef FilenameForLookup; + SmallString<256> PathBuf; + if (llvm::sys::path::is_absolute_gnu(OriginalFilename)) { +FilenameForLookup = OriginalFilename; + } else { +PathBuf = WorkingDirForCacheLookup; +llvm::sys::path::append(PathBuf, OriginalFilename); akyrtzi wrote: Maybe just check and remove a `./` prefix? `remove_dots()` can change the full path in a way it won't later match a given absolute path for the same file, so it's not clear it's always profitable. https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi review_requested https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi review_requested https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi created https://github.com/llvm/llvm-project/pull/66122: Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. >From 10b69de2194d918fda31cbe3c0a097520c73d072 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Tue, 12 Sep 2023 11:26:46 -0700 Subject: [PATCH] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths Previously a relative path would be used as a key for cache lookup and if the same relative path was used from another compiler invocation with a different working directory then the first cache entry was erroneously returned. --- .../DependencyScanningFilesystem.h| 18 - .../DependencyScanningFilesystem.cpp | 70 +++ clang/test/ClangScanDeps/relative-filenames.c | 38 ++ 3 files changed, 108 insertions(+), 18 deletions(-) create mode 100644 clang/test/ClangScanDeps/relative-filenames.c diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 4b4e3c7eb2ecd06..5d904c6437873b0 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -215,6 +215,7 @@ class DependencyScanningFilesystemLocalCache { public: /// Returns entry associated with the filename or nullptr if none is found. const CachedFileSystemEntry *findEntryByFilename(StringRef Filename) const { +assert(llvm::sys::path::is_absolute_gnu(Filename)); auto It = Cache.find(Filename); return It == Cache.end() ? nullptr : It->getValue(); } @@ -224,6 +225,7 @@ class DependencyScanningFilesystemLocalCache { const CachedFileSystemEntry & insertEntryForFilename(StringRef Filename, const CachedFileSystemEntry ) { +assert(llvm::sys::path::is_absolute_gnu(Filename)); const auto *InsertedEntry = Cache.insert({Filename, }).first->second; assert(InsertedEntry == && "entry already present"); return *InsertedEntry; @@ -282,13 +284,14 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { public: DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , - IntrusiveRefCntPtr FS) - : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache) {} + IntrusiveRefCntPtr FS); llvm::ErrorOr status(const Twine ) override; llvm::ErrorOr> openFileForRead(const Twine ) override; + std::error_code setCurrentWorkingDirectory(const Twine ) override; + /// Returns entry for the given filename. /// /// Attempts to use the local and shared caches first, then falls back to @@ -304,8 +307,11 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// For a filename that's not yet associated with any entry in the caches, /// uses the underlying filesystem to either look up the entry based in the /// shared cache indexed by unique ID, or creates new entry from scratch. + /// \p FilenameForLookup will always be an absolute path, and different than + /// \p OriginalFilename if \p OriginalFilename is relative. llvm::ErrorOr - computeAndStoreResult(StringRef Filename); + computeAndStoreResult(StringRef OriginalFilename, +StringRef FilenameForLookup); /// Scan for preprocessor directives for the given entry if necessary and /// returns a wrapper object with reference semantics. @@ -388,6 +394,12 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; + + /// The working directory to use for making relative paths absolute before + /// using them for cache lookups. + std::string WorkingDirForCacheLookup; + + void updateWorkingDirForCacheLookup(llvm::ErrorOr CWD); }; } // end namespace dependencies diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp index 31404855e3b1dc3..9f0daaed4fdf9ff 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp @@ -96,6 +96,7 @@ DependencyScanningFilesystemSharedCache:: DependencyScanningFilesystemSharedCache::CacheShard & DependencyScanningFilesystemSharedCache::getShardForFilename( StringRef Filename) const { + assert(llvm::sys::path::is_absolute_gnu(Filename)); return CacheShards[llvm::hash_value(Filename) % NumShards];
[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
https://github.com/akyrtzi review_requested https://github.com/llvm/llvm-project/pull/66122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 03a0f4b - [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files
Author: Argyrios Kyrtzidis Date: 2023-06-26T10:18:02-07:00 New Revision: 03a0f4b61ca50a267a405a29ff1986473a55f9d9 URL: https://github.com/llvm/llvm-project/commit/03a0f4b61ca50a267a405a29ff1986473a55f9d9 DIFF: https://github.com/llvm/llvm-project/commit/03a0f4b61ca50a267a405a29ff1986473a55f9d9.diff LOG: [clang/HeaderSearch] Make sure `loadSubdirectoryModuleMaps` doesn't cause loading of regular files `HeaderSearch::loadSubdirectoryModuleMaps` `stat`s all the files in a directory which causes the dependency scanning service to load and cache their contents. This is problematic because a file may be in the process of being generated and could be cached by the dep-scan service while it is still incomplete. To address this change `loadSubdirectoryModuleMaps` to ignore regular files. Differential Revision: https://reviews.llvm.org/D153670 Added: Modified: clang/lib/Lex/HeaderSearch.cpp clang/unittests/Tooling/DependencyScannerTest.cpp Removed: diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 723479ca4fbbc..6fe655fb24277 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -1917,6 +1917,8 @@ void HeaderSearch::loadSubdirectoryModuleMaps(DirectoryLookup ) { llvm::vfs::FileSystem = FileMgr.getVirtualFileSystem(); for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd; Dir != DirEnd && !EC; Dir.increment(EC)) { +if (Dir->type() == llvm::sys::fs::file_type::regular_file) + continue; bool IsFramework = llvm::sys::path::extension(Dir->path()) == ".framework"; if (IsFramework == SearchDir.isFramework()) loadModuleMapFile(Dir->path(), SearchDir.isSystemHeaderDirectory(), diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp index abcc2c787b0d0..8735fcad20046 100644 --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -239,3 +239,65 @@ TEST(DependencyScanner, ScanDepsWithFS) { EXPECT_EQ(convert_to_slash(DepFile), "test.cpp.o: /root/test.cpp /root/header.h\n"); } + +TEST(DependencyScanner, ScanDepsWithModuleLookup) { + std::vector CommandLine = { + "clang", + "-target", + "x86_64-apple-macosx10.7", + "-c", + "test.m", + "-o" + "test.m.o", + "-fmodules", + "-I/root/SomeSources", + }; + StringRef CWD = "/root"; + + auto VFS = new llvm::vfs::InMemoryFileSystem(); + VFS->setCurrentWorkingDirectory(CWD); + auto Sept = llvm::sys::path::get_separator(); + std::string OtherPath = + std::string(llvm::formatv("{0}root{0}SomeSources{0}other.h", Sept)); + std::string TestPath = std::string(llvm::formatv("{0}root{0}test.m", Sept)); + + VFS->addFile(OtherPath, 0, llvm::MemoryBuffer::getMemBuffer("\n")); + VFS->addFile(TestPath, 0, llvm::MemoryBuffer::getMemBuffer("@import Foo;\n")); + + struct InterceptorFS : llvm::vfs::ProxyFileSystem { +std::vector StatPaths; +std::vector ReadFiles; + +InterceptorFS(IntrusiveRefCntPtr UnderlyingFS) +: ProxyFileSystem(UnderlyingFS) {} + +llvm::ErrorOr status(const Twine ) override { + StatPaths.push_back(Path.str()); + return ProxyFileSystem::status(Path); +} + +llvm::ErrorOr> +openFileForRead(const Twine ) override { + ReadFiles.push_back(Path.str()); + return ProxyFileSystem::openFileForRead(Path); +} + }; + + auto InterceptFS = llvm::makeIntrusiveRefCnt(VFS); + + DependencyScanningService Service(ScanningMode::DependencyDirectivesScan, +ScanningOutputFormat::Make); + DependencyScanningTool ScanTool(Service, InterceptFS); + + // This will fail with "fatal error: module 'Foo' not found" but it doesn't + // matter, the point of the test is to check that files are not read + // unnecessarily. + std::string DepFile; + ASSERT_THAT_ERROR( + ScanTool.getDependencyFile(CommandLine, CWD).moveInto(DepFile), + llvm::Failed()); + + EXPECT_TRUE(llvm::find(InterceptFS->StatPaths, OtherPath) == + InterceptFS->StatPaths.end()); + EXPECT_EQ(InterceptFS->ReadFiles, std::vector{"test.m"}); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5e975d4 - [clang/Driver] Also consider `gnu++` standard when checking for modules support
Author: Argyrios Kyrtzidis Date: 2023-05-18T12:34:07-07:00 New Revision: 5e975d4f67c006420d0c65ccb0f5d08e3a352f46 URL: https://github.com/llvm/llvm-project/commit/5e975d4f67c006420d0c65ccb0f5d08e3a352f46 DIFF: https://github.com/llvm/llvm-project/commit/5e975d4f67c006420d0c65ccb0f5d08e3a352f46.diff LOG: [clang/Driver] Also consider `gnu++` standard when checking for modules support Differential Revision: https://reviews.llvm.org/D150473 Added: Modified: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/modules.cpp Removed: diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 3d55047ec478e..0a654c9393296 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3692,10 +3692,13 @@ static bool RenderModulesOptions(Compilation , const Driver , // modules support by default. bool HaveStdCXXModules = IsCXX && Std && - (Std->containsValue("c++2a") || Std->containsValue("c++20") || - Std->containsValue("c++2b") || Std->containsValue("c++23") || - Std->containsValue("c++2c") || Std->containsValue("c++26") || - Std->containsValue("c++latest")); + (Std->containsValue("c++2a") || Std->containsValue("gnu++2a") || + Std->containsValue("c++20") || Std->containsValue("gnu++20") || + Std->containsValue("c++2b") || Std->containsValue("gnu++2b") || + Std->containsValue("c++23") || Std->containsValue("gnu++23") || + Std->containsValue("c++2c") || Std->containsValue("gnu++2c") || + Std->containsValue("c++26") || Std->containsValue("gnu++26") || + Std->containsValue("c++latest") || Std->containsValue("gnu++latest")); bool HaveModules = HaveStdCXXModules; // -fmodules enables the use of precompiled modules (off by default). diff --git a/clang/test/Driver/modules.cpp b/clang/test/Driver/modules.cpp index e63c946770d37..b0d1f2280d254 100644 --- a/clang/test/Driver/modules.cpp +++ b/clang/test/Driver/modules.cpp @@ -4,6 +4,7 @@ // Check compiling a module interface to a .pcm file. // // RUN: %clang -std=c++2a -x c++-module --precompile %s -o %t/module.pcm -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE +// RUN: %clang -std=gnu++2a -x c++-module --precompile %s -o %t/module-gnu.pcm -v 2>&1 | FileCheck %s --check-prefix=CHECK-PRECOMPILE // // CHECK-PRECOMPILE: -cc1 {{.*}} -emit-module-interface // CHECK-PRECOMPILE-SAME: -o {{.*}}.pcm @@ -23,6 +24,7 @@ // // RUN: %clang -std=c++2a -fmodule-file=%t/module.pcm -Dexport= %s -S -o %t/module.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE // RUN: %clang -std=c++20 -fmodule-file=%t/module.pcm -Dexport= %s -S -o %t/module.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE +// RUN: %clang -std=gnu++20 -fmodule-file=%t/module-gnu.pcm -Dexport= %s -S -o %t/module.o -v 2>&1 | FileCheck %s --check-prefix=CHECK-USE // // CHECK-USE: -cc1 {{.*}} {{-emit-obj|-S}} // CHECK-USE-SAME: -fmodule-file={{.*}}.pcm ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b3283bf - [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78a` commit
Author: Argyrios Kyrtzidis Date: 2023-03-07T20:23:07-08:00 New Revision: b3283bf192c6dbd6416b58e42f90ca443e8f005d URL: https://github.com/llvm/llvm-project/commit/b3283bf192c6dbd6416b58e42f90ca443e8f005d DIFF: https://github.com/llvm/llvm-project/commit/b3283bf192c6dbd6416b58e42f90ca443e8f005d.diff LOG: [test/ARCMT/verify.m] Add lit test for `5e035651fd3acbb2645abbe80cae332d90eac78a` commit Differential Revision: https://reviews.llvm.org/D145473 Added: Modified: clang/test/ARCMT/verify.m Removed: diff --git a/clang/test/ARCMT/verify.m b/clang/test/ARCMT/verify.m index 9b46c1a729ac2..7480e0dc9677c 100644 --- a/clang/test/ARCMT/verify.m +++ b/clang/test/ARCMT/verify.m @@ -8,6 +8,9 @@ #error should not be ignored // expected-error@-1 {{should not be ignored}} +#error +// expected-error@-1 {{}} + // CHECK: error: no expected directives found: consider use of 'expected-no-diagnostics' // CHECK-NEXT: error: 'error' diagnostics seen but not expected: // CHECK-NEXT: (frontend): error reading '{{.*}}verify.m.tmp.invalid' ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5e03565 - [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string
Author: Argyrios Kyrtzidis Date: 2023-03-03T12:48:48-08:00 New Revision: 5e035651fd3acbb2645abbe80cae332d90eac78a URL: https://github.com/llvm/llvm-project/commit/5e035651fd3acbb2645abbe80cae332d90eac78a DIFF: https://github.com/llvm/llvm-project/commit/5e035651fd3acbb2645abbe80cae332d90eac78a.diff LOG: [clang/Diagnostic] Use `optional` to disambiguate between a `StoredDiagMessage` that is not set vs set as empty string "But when would you have a completely empty diagnostic message", you ask dear reader? That is when there is an empty "#warning" in code. rdar://106155415 Differential Revision: https://reviews.llvm.org/D145256 Added: Modified: clang/include/clang/Basic/Diagnostic.h clang/lib/Basic/Diagnostic.cpp clang/unittests/Basic/DiagnosticTest.cpp Removed: diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index b9ba459d1358b..5606a22fe9d68 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -1565,7 +1565,7 @@ inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) { /// currently in-flight diagnostic. class Diagnostic { const DiagnosticsEngine *DiagObj; - StringRef StoredDiagMessage; + std::optional StoredDiagMessage; public: explicit Diagnostic(const DiagnosticsEngine *DO) : DiagObj(DO) {} diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index dbe62ecb50d33..3474acbced195 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -793,8 +793,8 @@ static const char *getTokenDescForDiagnostic(tok::TokenKind Kind) { /// array. void Diagnostic:: FormatDiagnostic(SmallVectorImpl ) const { - if (!StoredDiagMessage.empty()) { -OutStr.append(StoredDiagMessage.begin(), StoredDiagMessage.end()); + if (StoredDiagMessage.has_value()) { +OutStr.append(StoredDiagMessage->begin(), StoredDiagMessage->end()); return; } diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp index f4ce7e187f8f2..7469019391716 100644 --- a/clang/unittests/Basic/DiagnosticTest.cpp +++ b/clang/unittests/Basic/DiagnosticTest.cpp @@ -9,6 +9,7 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticError.h" #include "clang/Basic/DiagnosticIDs.h" +#include "clang/Basic/DiagnosticLex.h" #include "gtest/gtest.h" #include @@ -128,4 +129,26 @@ TEST(DiagnosticTest, diagnosticError) { EXPECT_EQ(*Value, std::make_pair(20, 1)); EXPECT_EQ(Value->first, 20); } + +TEST(DiagnosticTest, storedDiagEmptyWarning) { + DiagnosticsEngine Diags(new DiagnosticIDs(), new DiagnosticOptions); + + class CaptureDiagnosticConsumer : public DiagnosticConsumer { + public: +SmallVector StoredDiags; + +void HandleDiagnostic(DiagnosticsEngine::Level level, + const Diagnostic ) override { + StoredDiags.push_back(StoredDiagnostic(level, Info)); +} + }; + + CaptureDiagnosticConsumer CaptureConsumer; + Diags.setClient(, /*ShouldOwnClient=*/false); + Diags.Report(diag::pp_hash_warning) << ""; + ASSERT_TRUE(CaptureConsumer.StoredDiags.size() == 1); + + // Make sure an empty warning can round-trip with \c StoredDiagnostic. + Diags.Report(CaptureConsumer.StoredDiags.front()); +} } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a93cb93 - [clang/driver] Make sure that `-gno-modules` by itself doesn't enable debug info
Author: Argyrios Kyrtzidis Date: 2023-01-21T11:33:30-08:00 New Revision: a93cb9302c1ac03f71d3a85de9405dc97c1bbc99 URL: https://github.com/llvm/llvm-project/commit/a93cb9302c1ac03f71d3a85de9405dc97c1bbc99 DIFF: https://github.com/llvm/llvm-project/commit/a93cb9302c1ac03f71d3a85de9405dc97c1bbc99.diff LOG: [clang/driver] Make sure that `-gno-modules` by itself doesn't enable debug info Added: Modified: clang/include/clang/Driver/Options.td clang/test/Driver/debug-options.c Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 12425e18406f0..40fb986fd6798 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3291,7 +3291,7 @@ def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group; def gmodules : Flag <["-"], "gmodules">, Group, HelpText<"Generate debug info with external references to clang modules" " or precompiled headers">; -def gno_modules : Flag <["-"], "gno-modules">, Group; +def gno_modules : Flag <["-"], "gno-modules">, Group; def gz_EQ : Joined<["-"], "gz=">, Group, HelpText<"DWARF debug sections compression type">; def gz : Flag<["-"], "gz">, Alias, AliasArgs<["zlib"]>, Group; diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c index b2198ebd0a207..dedd96c82da96 100644 --- a/clang/test/Driver/debug-options.c +++ b/clang/test/Driver/debug-options.c @@ -303,6 +303,8 @@ // RUN: %clang -### -gmodules -gno-modules %s 2>&1 \ // RUN:| FileCheck -check-prefix=NOGEXTREFS %s // +// RUN: %clang -### -gno-modules %s 2>&1 | FileCheck -check-prefix=GNOMOD %s +// // NOG_PS: "-cc1" // NOG_PS-NOT: "-dwarf-version= // NOG_PS: "-generate-arange-section" @@ -411,6 +413,8 @@ // GEXTREFS: "-dwarf-ext-refs" "-fmodule-format=obj" // GEXTREFS: "-debug-info-kind={{standalone|constructor}}" // NOGEXTREFS-NOT: -dwarf-ext-refs +// +// GNOMOD-NOT: -debug-info-kind= // RUN: not %clang -cc1 -debug-info-kind=watkind 2>&1 | FileCheck -check-prefix=BADSTRING1 %s // BADSTRING1: error: invalid value 'watkind' in '-debug-info-kind=watkind' ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b2b078a - [clang/CodeGenActionTest] Use the platform's path separator for the `DebugInfoCWDCodeGen` test
Author: Argyrios Kyrtzidis Date: 2023-01-20T13:20:21-08:00 New Revision: b2b078adc2d00db8dc8f0009e2bfca4d8267149a URL: https://github.com/llvm/llvm-project/commit/b2b078adc2d00db8dc8f0009e2bfca4d8267149a DIFF: https://github.com/llvm/llvm-project/commit/b2b078adc2d00db8dc8f0009e2bfca4d8267149a.diff LOG: [clang/CodeGenActionTest] Use the platform's path separator for the `DebugInfoCWDCodeGen` test Fixes a failure in some Windows configuration. Differential Revision: https://reviews.llvm.org/D142238 Added: Modified: clang/unittests/Frontend/CodeGenActionTest.cpp Removed: diff --git a/clang/unittests/Frontend/CodeGenActionTest.cpp b/clang/unittests/Frontend/CodeGenActionTest.cpp index d50648ced7daf..a6520910c8399 100644 --- a/clang/unittests/Frontend/CodeGenActionTest.cpp +++ b/clang/unittests/Frontend/CodeGenActionTest.cpp @@ -82,9 +82,10 @@ TEST(CodeGenTest, DebugInfoCWDCodeGen) { // Check that debug info is accessing the current working directory from the // VFS instead of calling \p llvm::sys::fs::current_path() directly. - auto VFS = std::make_unique(); - VFS->setCurrentWorkingDirectory("/in-memory-fs-cwd"); auto Sept = llvm::sys::path::get_separator(); + auto VFS = std::make_unique(); + VFS->setCurrentWorkingDirectory( + std::string(llvm::formatv("{0}in-memory-fs-cwd", Sept))); std::string TestPath = std::string(llvm::formatv("{0}in-memory-fs-cwd{0}test.cpp", Sept)); VFS->addFile(TestPath, 0, llvm::MemoryBuffer::getMemBuffer("int x;\n")); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1ff8a68 - [clang/driver] Add `-gno-modules` as the negative version of `-gmodules`
Author: Argyrios Kyrtzidis Date: 2023-01-20T12:39:07-08:00 New Revision: 1ff8a687ae151df72ffd3aac534fdd0d1b39f7ca URL: https://github.com/llvm/llvm-project/commit/1ff8a687ae151df72ffd3aac534fdd0d1b39f7ca DIFF: https://github.com/llvm/llvm-project/commit/1ff8a687ae151df72ffd3aac534fdd0d1b39f7ca.diff LOG: [clang/driver] Add `-gno-modules` as the negative version of `-gmodules` Differential Revision: https://reviews.llvm.org/D142236 Added: Modified: clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/debug-options.c Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 343cc77a18c4..a100d2c3e502 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3292,6 +3292,7 @@ def gdwarf_aranges : Flag<["-"], "gdwarf-aranges">, Group; def gmodules : Flag <["-"], "gmodules">, Group, HelpText<"Generate debug info with external references to clang modules" " or precompiled headers">; +def gno_modules : Flag <["-"], "gno-modules">, Group; def gz_EQ : Joined<["-"], "gz=">, Group, HelpText<"DWARF debug sections compression type">; def gz : Flag<["-"], "gz">, Alias, AliasArgs<["zlib"]>, Group; diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index f766eeda3cc4..d4a0772d6149 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4219,9 +4219,11 @@ static void renderDebugOptions(const ToolChain , const Driver , CmdArgs.push_back("-gno-column-info"); // FIXME: Move backend command line options to the module. - // If -gline-tables-only or -gline-directives-only is the last option it wins. - if (const Arg *A = Args.getLastArg(options::OPT_gmodules)) -if (checkDebugInfoOption(A, Args, D, TC)) { + if (Args.hasFlag(options::OPT_gmodules, options::OPT_gno_modules, false)) { +// If -gline-tables-only or -gline-directives-only is the last option it +// wins. +if (checkDebugInfoOption(Args.getLastArg(options::OPT_gmodules), Args, D, + TC)) { if (DebugInfoKind != codegenoptions::DebugLineTablesOnly && DebugInfoKind != codegenoptions::DebugDirectivesOnly) { DebugInfoKind = codegenoptions::DebugInfoConstructor; @@ -4229,6 +4231,7 @@ static void renderDebugOptions(const ToolChain , const Driver , CmdArgs.push_back("-fmodule-format=obj"); } } + } if (T.isOSBinFormatELF() && SplitDWARFInlining) CmdArgs.push_back("-fsplit-dwarf-inlining"); diff --git a/clang/test/Driver/debug-options.c b/clang/test/Driver/debug-options.c index 37a975da5b48..b2198ebd0a20 100644 --- a/clang/test/Driver/debug-options.c +++ b/clang/test/Driver/debug-options.c @@ -300,6 +300,9 @@ // RUN: %clang -### -target %itanium_abi_triple -gmodules -gline-directives-only %s 2>&1 \ // RUN:| FileCheck -check-prefix=GLIO_ONLY %s // +// RUN: %clang -### -gmodules -gno-modules %s 2>&1 \ +// RUN:| FileCheck -check-prefix=NOGEXTREFS %s +// // NOG_PS: "-cc1" // NOG_PS-NOT: "-dwarf-version= // NOG_PS: "-generate-arange-section" @@ -407,6 +410,7 @@ // // GEXTREFS: "-dwarf-ext-refs" "-fmodule-format=obj" // GEXTREFS: "-debug-info-kind={{standalone|constructor}}" +// NOGEXTREFS-NOT: -dwarf-ext-refs // RUN: not %clang -cc1 -debug-info-kind=watkind 2>&1 | FileCheck -check-prefix=BADSTRING1 %s // BADSTRING1: error: invalid value 'watkind' in '-debug-info-kind=watkind' ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ed6d09d - [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals
Author: Argyrios Kyrtzidis Date: 2023-01-19T15:23:21-08:00 New Revision: ed6d09dd4ead70d2858d56c530af38eefa1ef595 URL: https://github.com/llvm/llvm-project/commit/ed6d09dd4ead70d2858d56c530af38eefa1ef595 DIFF: https://github.com/llvm/llvm-project/commit/ed6d09dd4ead70d2858d56c530af38eefa1ef595.diff LOG: [Lex] For dependency directive lexing, angled includes in `__has_include` should be lexed as string literals rdar://104386604 Differential Revision: https://reviews.llvm.org/D142143 Added: clang/test/ClangScanDeps/depscan-lex-has-include.c Modified: clang/lib/Lex/Lexer.cpp Removed: diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp index 578bd69ddbfbc..d49d9e9e4b142 100644 --- a/clang/lib/Lex/Lexer.cpp +++ b/clang/lib/Lex/Lexer.cpp @@ -4408,6 +4408,22 @@ bool Lexer::LexDependencyDirectiveToken(Token ) { MIOpt.ReadToken(); } + if (ParsingFilename && DDTok.is(tok::less)) { +BufferPtr = BufferStart + DDTok.Offset; +LexAngledStringLiteral(Result, BufferPtr + 1); +if (Result.isNot(tok::header_name)) + return true; +// Advance the index of lexed tokens. +while (true) { + const dependency_directives_scan::Token = + DepDirectives.front().Tokens[NextDepDirectiveTokenIndex]; + if (BufferStart + NextTok.Offset >= BufferPtr) +break; + ++NextDepDirectiveTokenIndex; +} +return true; + } + const char *TokPtr = convertDependencyDirectiveToken(DDTok, Result); if (Result.is(tok::hash) && Result.isAtStartOfLine()) { diff --git a/clang/test/ClangScanDeps/depscan-lex-has-include.c b/clang/test/ClangScanDeps/depscan-lex-has-include.c new file mode 100644 index 0..4d46e8d7d19e2 --- /dev/null +++ b/clang/test/ClangScanDeps/depscan-lex-has-include.c @@ -0,0 +1,46 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json + +// RUN: clang-scan-deps -compilation-database %t/cdb.json | FileCheck %s +// CHECK: t.c +// CHECK: something.h + +// RUN: sed -e "s|DIR|%/t|g" %t/cdb-error.json.template > %t/cdb-error.json +// RUN: not clang-scan-deps -compilation-database %t/cdb-error.json 2>&1 | FileCheck %s -check-prefix=ERROR +// ERROR: error: expected '>' +// ERROR: error: expected value in expression + +//--- cdb.json.template +[ + { +"directory": "DIR", +"command": "clang -fsyntax-only DIR/t.c -I DIR", +"file": "DIR/t.c" + } +] + +//--- cdb-error.json.template +[ + { +"directory": "DIR", +"command": "clang -fsyntax-only DIR/error.c", +"file": "DIR/error.c" + } +] + +//--- t.c + +#define something + +// Make sure the include is lexed as a literal, ignoring the macro. +#if __has_include() +#include +#endif + +//--- something/something.h + +//--- error.c +#if __has_include(https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 59df564 - [clang/Lexer] Enhance `Lexer::getImmediateMacroNameForDiagnostics` to return a result from non-file buffers
Author: Argyrios Kyrtzidis Date: 2022-12-15T22:46:41-08:00 New Revision: 59df56413bdc25bef53bf1629b26bd2176089088 URL: https://github.com/llvm/llvm-project/commit/59df56413bdc25bef53bf1629b26bd2176089088 DIFF: https://github.com/llvm/llvm-project/commit/59df56413bdc25bef53bf1629b26bd2176089088.diff LOG: [clang/Lexer] Enhance `Lexer::getImmediateMacroNameForDiagnostics` to return a result from non-file buffers Use `SourceManager::isWrittenInScratchSpace()` to specifically check for token paste or stringization, instead of excluding all non-file buffers. This allows diagnostics to mention macro names that were defined from the command-line. Differential Revision: https://reviews.llvm.org/D140164 Added: Modified: clang/lib/Lex/Lexer.cpp clang/test/Modules/build-fail-notes.m clang/test/Modules/epic-fail.m Removed: diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp index 3866c2c85f18e..4a9c5b5cd680e 100644 --- a/clang/lib/Lex/Lexer.cpp +++ b/clang/lib/Lex/Lexer.cpp @@ -1047,9 +1047,11 @@ StringRef Lexer::getImmediateMacroNameForDiagnostics( while (SM.isMacroArgExpansion(Loc)) Loc = SM.getImmediateExpansionRange(Loc).getBegin(); - // If the macro's spelling has no FileID, then it's actually a token paste - // or stringization (or similar) and not a macro at all. - if (!SM.getFileEntryForID(SM.getFileID(SM.getSpellingLoc(Loc + // If the macro's spelling isn't FileID or from scratch space, then it's + // actually a token paste or stringization (or similar) and not a macro at + // all. + SourceLocation SpellLoc = SM.getSpellingLoc(Loc); + if (!SpellLoc.isFileID() || SM.isWrittenInScratchSpace(SpellLoc)) return {}; // Find the spelling location of the start of the non-argument expansion diff --git a/clang/test/Modules/build-fail-notes.m b/clang/test/Modules/build-fail-notes.m index 47bdbc7fca6cb..929e1f1890e8a 100644 --- a/clang/test/Modules/build-fail-notes.m +++ b/clang/test/Modules/build-fail-notes.m @@ -6,7 +6,7 @@ // CHECK: While building module 'DependsOnModule' imported from // CHECK: While building module 'Module' imported from // CHECK: error: expected ';' after top level declarator -// CHECK: note: expanded from here +// CHECK: note: expanded from macro 'getModuleVersion' // CHECK: fatal error: could not build module 'Module' // CHECK: fatal error: could not build module 'DependsOnModule' // CHECK-NOT: error: @@ -24,7 +24,7 @@ // CHECK-SDIAG: Module.h:9:13: error: expected ';' after top level declarator // CHECK-SDIAG: build-fail-notes.m:4:9: note: while building module 'DependsOnModule' imported from // CHECK-SDIAG: DependsOnModule.h:1:10: note: while building module 'Module' imported from -// CHECK-SDIAG: note: expanded from here +// CHECK-SDIAG: note: expanded from macro 'getModuleVersion' // CHECK-SDIAG: warning: umbrella header for module 'Module' does not include header 'NotInModule.h' [-Wincomplete-umbrella] // CHECK-SDIAG: DependsOnModule.h:1:10: fatal: could not build module 'Module' // CHECK-SDIAG: build-fail-notes.m:4:9: note: while building module 'DependsOnModule' imported from diff --git a/clang/test/Modules/epic-fail.m b/clang/test/Modules/epic-fail.m index b368ceaec8984..efa397b0a8497 100644 --- a/clang/test/Modules/epic-fail.m +++ b/clang/test/Modules/epic-fail.m @@ -6,7 +6,7 @@ // CHECK: While building module 'Module' imported from // CHECK: error: expected ';' after top level declarator -// CHECK: note: expanded from here +// CHECK: note: expanded from macro 'getModuleVersion' // CHECK: fatal error: could not build module 'Module' // CHECK: While building module 'DependsOnModule' imported from // CHECK: fatal error: could not build module 'Module' ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0456acb - [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic
Author: Argyrios Kyrtzidis Date: 2022-10-11T13:39:26-07:00 New Revision: 0456acbfb942f127359a8defd1b4f1f44420df3e URL: https://github.com/llvm/llvm-project/commit/0456acbfb942f127359a8defd1b4f1f44420df3e DIFF: https://github.com/llvm/llvm-project/commit/0456acbfb942f127359a8defd1b4f1f44420df3e.diff LOG: [clang/Sema] Follow-up for fix of non-deterministic order of `-Wunused-variable` diagnostic Commit `371883f46dc23f8464cbf578e2d12a4f92e61917` caused a noticeable compile-time regression (about 0.4% geomean at -O0): http://llvm-compile-time-tracker.com/compare.php?from=92233159035d1b50face95d886901cf99035bd99=371883f46dc23f8464cbf578e2d12a4f92e61917=instructions To address this switch `Scope::DeclSetTy` back to a `SmallPtrSet` and change `Sema::ActOnPopScope` to explicitly order the diagnostics that this function emits. Differential Revision: https://reviews.llvm.org/D135490 Added: Modified: clang/include/clang/Sema/Scope.h clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDecl.cpp Removed: diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h index 04ab416bf7352..d2acf253552dc 100644 --- a/clang/include/clang/Sema/Scope.h +++ b/clang/include/clang/Sema/Scope.h @@ -16,7 +16,6 @@ #include "clang/AST/Decl.h" #include "clang/Basic/Diagnostic.h" #include "llvm/ADT/PointerIntPair.h" -#include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/iterator_range.h" @@ -197,7 +196,7 @@ class Scope { /// popped, these declarations are removed from the IdentifierTable's notion /// of current declaration. It is up to the current Action implementation to /// implement these semantics. - using DeclSetTy = llvm::SmallSetVector; + using DeclSetTy = llvm::SmallPtrSet; DeclSetTy DeclsInScope; /// The DeclContext with which this scope is associated. For @@ -322,7 +321,7 @@ class Scope { DeclsInScope.insert(D); } - void RemoveDecl(Decl *D) { DeclsInScope.remove(D); } + void RemoveDecl(Decl *D) { DeclsInScope.erase(D); } void incrementMSManglingNumber() { if (Scope *MSLMP = getMSLastManglingParent()) { @@ -350,9 +349,7 @@ class Scope { /// isDeclScope - Return true if this is the scope that the specified decl is /// declared in. - bool isDeclScope(const Decl *D) const { -return DeclsInScope.contains(const_cast(D)); - } + bool isDeclScope(const Decl *D) const { return DeclsInScope.contains(D); } /// Get the entity corresponding to this scope. DeclContext *getEntity() const { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a6acdf6b49526..405f84f0b5854 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5221,15 +5221,21 @@ class Sema final { /// of it. void MarkUnusedFileScopedDecl(const DeclaratorDecl *D); + typedef llvm::function_ref + DiagReceiverTy; + /// DiagnoseUnusedExprResult - If the statement passed in is an expression /// whose result is unused, warn. void DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID); void DiagnoseUnusedNestedTypedefs(const RecordDecl *D); + void DiagnoseUnusedNestedTypedefs(const RecordDecl *D, +DiagReceiverTy DiagReceiver); void DiagnoseUnusedDecl(const NamedDecl *ND); + void DiagnoseUnusedDecl(const NamedDecl *ND, DiagReceiverTy DiagReceiver); /// If VD is set but not otherwise used, diagnose, for a parameter or a /// variable. - void DiagnoseUnusedButSetDecl(const VarDecl *VD); + void DiagnoseUnusedButSetDecl(const VarDecl *VD, DiagReceiverTy DiagReceiver); /// Emit \p DiagID if statement located on \p StmtLoc has a suspicious null /// statement as a \p Body, and it is located on the same line. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 1bf959a35178a..5cc3db63a1996 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2091,20 +2091,31 @@ static void GenerateFixForUnusedDecl(const NamedDecl *D, ASTContext , } void Sema::DiagnoseUnusedNestedTypedefs(const RecordDecl *D) { + DiagnoseUnusedNestedTypedefs( + D, [this](SourceLocation Loc, PartialDiagnostic PD) { Diag(Loc, PD); }); +} + +void Sema::DiagnoseUnusedNestedTypedefs(const RecordDecl *D, +DiagReceiverTy DiagReceiver) { if (D->getTypeForDecl()->isDependentType()) return; for (auto *TmpD : D->decls()) { if (const auto *T = dyn_cast(TmpD)) - DiagnoseUnusedDecl(T); + DiagnoseUnusedDecl(T, DiagReceiver); else if(const auto *R = dyn_cast(TmpD)) - DiagnoseUnusedNestedTypedefs(R); + DiagnoseUnusedNestedTypedefs(R, DiagReceiver); } } +void Sema::DiagnoseUnusedDecl(const NamedDecl *D) { + DiagnoseUnusedDecl( + D, [this](SourceLocation Loc,
[clang] 371883f - [clang/Sema] Fix non-deterministic order for certain kind of diagnostics
Author: Argyrios Kyrtzidis Date: 2022-10-05T12:58:01-07:00 New Revision: 371883f46dc23f8464cbf578e2d12a4f92e61917 URL: https://github.com/llvm/llvm-project/commit/371883f46dc23f8464cbf578e2d12a4f92e61917 DIFF: https://github.com/llvm/llvm-project/commit/371883f46dc23f8464cbf578e2d12a4f92e61917.diff LOG: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics In the context of caching clang invocations it is important to emit diagnostics in deterministic order; the same clang invocation should result in the same diagnostic output. rdar://100336989 Differential Revision: https://reviews.llvm.org/D135118 Added: clang/test/Sema/deterministic-diagnostics-order.m Modified: clang/include/clang/AST/DeclObjC.h clang/include/clang/Sema/Scope.h clang/lib/AST/DeclObjC.cpp clang/lib/Sema/SemaObjCProperty.cpp clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp Removed: diff --git a/clang/include/clang/AST/DeclObjC.h b/clang/include/clang/AST/DeclObjC.h index 59a2cf5de234..210b7adebe4c 100644 --- a/clang/include/clang/AST/DeclObjC.h +++ b/clang/include/clang/AST/DeclObjC.h @@ -25,8 +25,8 @@ #include "clang/Basic/SourceLocation.h" #include "clang/Basic/Specifiers.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/MapVector.h" #include "llvm/ADT/None.h" #include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/STLExtras.h" @@ -1079,16 +1079,15 @@ class ObjCContainerDecl : public NamedDecl, public DeclContext { ObjCPropertyQueryKind QueryKind) const; using PropertyMap = - llvm::DenseMap, - ObjCPropertyDecl *>; + llvm::MapVector, + ObjCPropertyDecl *>; using ProtocolPropertySet = llvm::SmallDenseSet; using PropertyDeclOrder = llvm::SmallVector; /// This routine collects list of properties to be implemented in the class. /// This includes, class's and its conforming protocols' properties. /// Note, the superclass's properties are not included in the list. - virtual void collectPropertiesToImplement(PropertyMap , -PropertyDeclOrder ) const {} + virtual void collectPropertiesToImplement(PropertyMap ) const {} SourceLocation getAtStartLoc() const { return ObjCContainerDeclBits.AtStart; } @@ -1780,8 +1779,7 @@ class ObjCInterfaceDecl : public ObjCContainerDecl *FindPropertyVisibleInPrimaryClass(IdentifierInfo *PropertyId, ObjCPropertyQueryKind QueryKind) const; - void collectPropertiesToImplement(PropertyMap , -PropertyDeclOrder ) const override; + void collectPropertiesToImplement(PropertyMap ) const override; /// isSuperClassOf - Return true if this class is the specified class or is a /// super class of the specified interface class. @@ -2246,8 +2244,7 @@ class ObjCProtocolDecl : public ObjCContainerDecl, ObjCProtocolDecl *getCanonicalDecl() override { return getFirstDecl(); } const ObjCProtocolDecl *getCanonicalDecl() const { return getFirstDecl(); } - void collectPropertiesToImplement(PropertyMap , -PropertyDeclOrder ) const override; + void collectPropertiesToImplement(PropertyMap ) const override; void collectInheritedProtocolProperties(const ObjCPropertyDecl *Property, ProtocolPropertySet , diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h index 3749d925b106..04ab416bf735 100644 --- a/clang/include/clang/Sema/Scope.h +++ b/clang/include/clang/Sema/Scope.h @@ -16,6 +16,7 @@ #include "clang/AST/Decl.h" #include "clang/Basic/Diagnostic.h" #include "llvm/ADT/PointerIntPair.h" +#include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/iterator_range.h" @@ -196,7 +197,7 @@ class Scope { /// popped, these declarations are removed from the IdentifierTable's notion /// of current declaration. It is up to the current Action implementation to /// implement these semantics. - using DeclSetTy = llvm::SmallPtrSet; + using DeclSetTy = llvm::SmallSetVector; DeclSetTy DeclsInScope; /// The DeclContext with which this scope is associated. For @@ -321,9 +322,7 @@ class Scope { DeclsInScope.insert(D); } - void RemoveDecl(Decl *D) { -DeclsInScope.erase(D); - } + void RemoveDecl(Decl *D) { DeclsInScope.remove(D); } void incrementMSManglingNumber() { if (Scope *MSLMP = getMSLastManglingParent()) { @@ -351,7 +350,9 @@ class Scope { /// isDeclScope - Return true if this is the scope that the specified decl is /// declared in. - bool isDeclScope(const Decl *D) const { return DeclsInScope.contains(D); } + bool isDeclScope(const Decl
[clang] b340c5a - [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`
Author: Argyrios Kyrtzidis Date: 2022-09-13T15:48:50-07:00 New Revision: b340c5ae4221a9752712621cd1df06cbc6dfd50b URL: https://github.com/llvm/llvm-project/commit/b340c5ae4221a9752712621cd1df06cbc6dfd50b DIFF: https://github.com/llvm/llvm-project/commit/b340c5ae4221a9752712621cd1df06cbc6dfd50b.diff LOG: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash` Differential Revision: https://reviews.llvm.org/D133674 Added: Modified: clang/lib/Lex/DependencyDirectivesScanner.cpp clang/unittests/Lex/DependencyDirectivesScannerTest.cpp Removed: diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp index c0b1687b2847f..5d280079b882e 100644 --- a/clang/lib/Lex/DependencyDirectivesScanner.cpp +++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp @@ -742,6 +742,14 @@ bool Scanner::lexPPLine(const char *, const char *const End) { // Lex '#'. const dependency_directives_scan::Token = lexToken(First, End); + if (HashTok.is(tok::hashhash)) { +// A \p tok::hashhash at this location is passed by the preprocessor to the +// parser to interpret, like any other token. So for dependency scanning +// skip it like a normal token not affecting the preprocessor. +skipLine(First, End); +assert(First <= End); +return false; + } assert(HashTok.is(tok::hash)); (void)HashTok; diff --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp index 7ff09dc5d9c5b..5222df9fb9eb2 100644 --- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp +++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp @@ -124,6 +124,21 @@ TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) { EXPECT_STREQ("#define MACRO a\n", Out.data()); } +TEST(MinimizeSourceToDependencyDirectivesTest, HashHash) { + SmallVector Out; + + StringRef Source = R"( +#define S +#if 0 + ##pragma cool + ##include "t.h" +#endif +#define E +)"; + ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out)); + EXPECT_STREQ("#define S\n#if 0\n#endif\n#define E\n", Out.data()); +} + TEST(MinimizeSourceToDependencyDirectivesTest, Define) { SmallVector Out; SmallVector Tokens; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] aa484c9 - [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF
Author: Argyrios Kyrtzidis Date: 2022-09-07T10:31:29-07:00 New Revision: aa484c90cf5902042cec0f6a4f3bf2a460eea307 URL: https://github.com/llvm/llvm-project/commit/aa484c90cf5902042cec0f6a4f3bf2a460eea307 DIFF: https://github.com/llvm/llvm-project/commit/aa484c90cf5902042cec0f6a4f3bf2a460eea307.diff LOG: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF Directive `dependency_directives_scan::tokens_present_before_eof` is introduced to indicate there were tokens present before the last scanned dependency directive and EOF. This is useful to ensure we correctly identify the macro guards when lexing using the dependency directives. Differential Revision: https://reviews.llvm.org/D133357 Added: clang/unittests/Lex/PPDependencyDirectivesTest.cpp Modified: clang/include/clang/Lex/DependencyDirectivesScanner.h clang/lib/Lex/DependencyDirectivesScanner.cpp clang/lib/Lex/Lexer.cpp clang/unittests/Lex/CMakeLists.txt clang/unittests/Lex/DependencyDirectivesScannerTest.cpp Removed: diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h b/clang/include/clang/Lex/DependencyDirectivesScanner.h index ec6ac6173e855..529b93aa0ffbf 100644 --- a/clang/include/clang/Lex/DependencyDirectivesScanner.h +++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h @@ -82,6 +82,9 @@ enum DirectiveKind : uint8_t { cxx_import_decl, cxx_export_module_decl, cxx_export_import_decl, + /// Indicates that there are tokens present between the last scanned directive + /// and eof. The \p Directive::Tokens array will be empty for this kind. + tokens_present_before_eof, pp_eof, }; diff --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp b/clang/lib/Lex/DependencyDirectivesScanner.cpp index 0efcc352f0883..c0b1687b2847f 100644 --- a/clang/lib/Lex/DependencyDirectivesScanner.cpp +++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp @@ -87,6 +87,9 @@ struct Scanner { dependency_directives_scan::Token (const char *, const char *const End); + void skipLine(const char *, const char *const End); + void skipDirective(StringRef Name, const char *, const char *const End); + /// Lexes next token and if it is identifier returns its string, otherwise /// it skips the current line and returns \p None. /// @@ -150,6 +153,7 @@ struct Scanner { DiagnosticsEngine *Diags; SourceLocation InputSourceLoc; + const char *LastTokenPtr = nullptr; /// Keeps track of the tokens for the currently lexed directive. Once a /// directive is fully lexed and "committed" then the tokens get appended to /// \p Tokens and \p CurDirToks is cleared for the next directive. @@ -364,7 +368,7 @@ static bool isQuoteCppDigitSeparator(const char *const Start, return (Cur + 1) < End && isAsciiIdentifierContinue(*(Cur + 1)); } -static void skipLine(const char *, const char *const End) { +void Scanner::skipLine(const char *, const char *const End) { for (;;) { assert(First <= End); if (First == End) @@ -379,6 +383,7 @@ static void skipLine(const char *, const char *const End) { // Iterate over strings correctly to avoid comments and newlines. if (*First == '"' || (*First == '\'' && !isQuoteCppDigitSeparator(Start, First, End))) { +LastTokenPtr = First; if (isRawStringLiteral(Start, First)) skipRawString(First, End); else @@ -388,6 +393,7 @@ static void skipLine(const char *, const char *const End) { // Iterate over comments correctly. if (*First != '/' || End - First < 2) { +LastTokenPtr = First; ++First; continue; } @@ -399,6 +405,7 @@ static void skipLine(const char *, const char *const End) { } if (First[1] != '*') { +LastTokenPtr = First; ++First; continue; } @@ -416,8 +423,8 @@ static void skipLine(const char *, const char *const End) { } } -static void skipDirective(StringRef Name, const char *, - const char *const End) { +void Scanner::skipDirective(StringRef Name, const char *, +const char *const End) { if (llvm::StringSwitch(Name) .Case("warning", true) .Case("error", true) @@ -710,6 +717,8 @@ bool Scanner::lexPPLine(const char *, const char *const End) { return false; } + LastTokenPtr = First; + TheLexer.seek(getOffsetAt(First), /*IsAtStartOfLine*/ true); auto ScEx1 = make_scope_exit([&]() { @@ -803,6 +812,9 @@ bool Scanner::scan(SmallVectorImpl ) { if (!Error) { // Add an EOF on success. +if (LastTokenPtr && +(Tokens.empty() || LastTokenPtr > Input.begin() + Tokens.back().Offset)) + pushDirective(tokens_present_before_eof); pushDirective(pp_eof); } @@
[clang] 33162a8 - [driver] Additional ignoring of module-map related flags, if modules are disabled
Author: Argyrios Kyrtzidis Date: 2022-08-29T17:38:00-07:00 New Revision: 33162a81d4c93a53ef847d3601b0b03830937d3c URL: https://github.com/llvm/llvm-project/commit/33162a81d4c93a53ef847d3601b0b03830937d3c DIFF: https://github.com/llvm/llvm-project/commit/33162a81d4c93a53ef847d3601b0b03830937d3c.diff LOG: [driver] Additional ignoring of module-map related flags, if modules are disabled Differential Revision: https://reviews.llvm.org/D132801 Added: Modified: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/modules.m Removed: diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 1fa83e7121c95..401333a39025f 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3726,25 +3726,29 @@ static void RenderModulesOptions(Compilation , const Driver , options::OPT_fno_modules_validate_input_files_content, false)) CmdArgs.push_back("-fvalidate-ast-input-files-content"); - } - - // -fmodule-name specifies the module that is currently being built (or - // used for header checking by -fmodule-maps). - Args.AddLastArg(CmdArgs, options::OPT_fmodule_name_EQ); - // -fmodule-map-file can be used to specify files containing module - // definitions. - Args.AddAllArgs(CmdArgs, options::OPT_fmodule_map_file); - - // -fbuiltin-module-map can be used to load the clang - // builtin headers modulemap file. - if (Args.hasArg(options::OPT_fbuiltin_module_map)) { -SmallString<128> BuiltinModuleMap(D.ResourceDir); -llvm::sys::path::append(BuiltinModuleMap, "include"); -llvm::sys::path::append(BuiltinModuleMap, "module.modulemap"); -if (llvm::sys::fs::exists(BuiltinModuleMap)) - CmdArgs.push_back( - Args.MakeArgString("-fmodule-map-file=" + BuiltinModuleMap)); +// -fmodule-name specifies the module that is currently being built (or +// used for header checking by -fmodule-maps). +Args.AddLastArg(CmdArgs, options::OPT_fmodule_name_EQ); + +// -fmodule-map-file can be used to specify files containing module +// definitions. +Args.AddAllArgs(CmdArgs, options::OPT_fmodule_map_file); + +// -fbuiltin-module-map can be used to load the clang +// builtin headers modulemap file. +if (Args.hasArg(options::OPT_fbuiltin_module_map)) { + SmallString<128> BuiltinModuleMap(D.ResourceDir); + llvm::sys::path::append(BuiltinModuleMap, "include"); + llvm::sys::path::append(BuiltinModuleMap, "module.modulemap"); + if (llvm::sys::fs::exists(BuiltinModuleMap)) +CmdArgs.push_back( +Args.MakeArgString("-fmodule-map-file=" + BuiltinModuleMap)); +} + } else { +Args.ClaimAllArgs(options::OPT_fmodule_name_EQ); +Args.ClaimAllArgs(options::OPT_fmodule_map_file); +Args.ClaimAllArgs(options::OPT_fbuiltin_module_map); } // The -fmodule-file== form specifies the mapping of module diff --git a/clang/test/Driver/modules.m b/clang/test/Driver/modules.m index 4b67f7c7bec4f..7eaea1da5649f 100644 --- a/clang/test/Driver/modules.m +++ b/clang/test/Driver/modules.m @@ -75,8 +75,8 @@ // RUN: %clang -fno-modules -fbuild-session-timestamp=123 -### %s 2>&1 | FileCheck -check-prefix=SESSION_FLAG %s // SESSION_FLAG-NOT: -fbuild-session-timestamp -// RUN: %clang -fno-modules -fmodules-validate-once-per-build-session -### %s 2>&1 | FileCheck -check-prefix=VALIDATE_ONCE_FLAG %s -// VALIDATE_ONCE_FLAG-NOT: -fmodules-validate-once-per-build-session - -// RUN: %clang -fno-modules -fmodules-validate-system-headers -### %s 2>&1 | FileCheck -check-prefix=VALIDATE_SYSTEM_FLAG %s -// VALIDATE_SYSTEM_FLAG-NOT: -fmodules-validate-system-headers +// RUN: %clang -fno-modules -fmodules-validate-once-per-build-session -fmodules-validate-system-headers -fmodule-map-file=module.modulemap \ +// RUN: -### %s 2>&1 | FileCheck -check-prefix=IGNORED_FLAGS %s +// IGNORED_FLAGS-NOT: -fmodules-validate-once-per-build-session +// IGNORED_FLAGS-NOT: -fmodules-validate-system-headers +// IGNORED_FLAGS-NOT: -fmodule-map-file ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 7b12e56 - [test/Modules/cxx20-export-import.cpp] Pre-clean the modules cache directory of the test, NFC
Author: Argyrios Kyrtzidis Date: 2022-08-05T17:27:43-07:00 New Revision: 7b12e561ac7aa9604022d56c8561bd472e78f2ed URL: https://github.com/llvm/llvm-project/commit/7b12e561ac7aa9604022d56c8561bd472e78f2ed DIFF: https://github.com/llvm/llvm-project/commit/7b12e561ac7aa9604022d56c8561bd472e78f2ed.diff LOG: [test/Modules/cxx20-export-import.cpp] Pre-clean the modules cache directory of the test, NFC Added: Modified: clang/test/Modules/cxx20-export-import.cpp Removed: diff --git a/clang/test/Modules/cxx20-export-import.cpp b/clang/test/Modules/cxx20-export-import.cpp index a2620bd60064..6424ed5cebe3 100644 --- a/clang/test/Modules/cxx20-export-import.cpp +++ b/clang/test/Modules/cxx20-export-import.cpp @@ -1,3 +1,4 @@ +// RUN: rm -rf %t // RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify %s export import dummy; // expected-error {{export declaration can only be used within a module interface unit after the module declaration}} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6635f48 - [Serialization] Remove `ORIGINAL_PCH_DIR` record
Author: Argyrios Kyrtzidis Date: 2022-08-05T15:40:33-07:00 New Revision: 6635f48e4aba499a7a31c6346cb1351437d36055 URL: https://github.com/llvm/llvm-project/commit/6635f48e4aba499a7a31c6346cb1351437d36055 DIFF: https://github.com/llvm/llvm-project/commit/6635f48e4aba499a7a31c6346cb1351437d36055.diff LOG: [Serialization] Remove `ORIGINAL_PCH_DIR` record Use of `ORIGINAL_PCH_DIR` record has been superseeded by making PCH/PCM files with relocatable paths at write time. Removing this record is useful for producing an output-path-independent PCH file and enable sharing of the same PCH file even when it was intended for a different output path. Differential Revision: https://reviews.llvm.org/D131124 Added: Modified: clang/include/clang/Driver/Options.td clang/include/clang/Frontend/FrontendOptions.h clang/include/clang/Serialization/ASTBitCodes.h clang/include/clang/Serialization/ASTWriter.h clang/include/clang/Serialization/ModuleFile.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/GeneratePCH.cpp clang/test/PCH/pch-output-path-independent.c Removed: clang/test/Modules/relative-original-dir.m diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index eaf8406d306f3..abd74103f61c3 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6018,9 +6018,6 @@ def fno_validate_pch : Flag<["-"], "fno-validate-pch">, HelpText<"Disable validation of precompiled headers">, MarshallingInfoFlag, "DisableValidationForModuleKind::None">, Normalizer<"makeFlagToValueNormalizer(DisableValidationForModuleKind::All)">; -def fpcm_output_path_independent : Flag<["-"], "fpcm-output-path-independent">, - HelpText<"Output a PCM/PCH file that does not write out information about its output path">, - MarshallingInfoFlag>; def fallow_pcm_with_errors : Flag<["-"], "fallow-pcm-with-compiler-errors">, HelpText<"Accept a PCM file that was created with compiler errors">, MarshallingInfoFlag>; diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 56e704d8a52f4..4f9aa1a749c96 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -344,15 +344,6 @@ class FrontendOptions { /// When using -emit-module, treat the modulemap as a system module. unsigned IsSystemModule : 1; - /// Output a PCM/PCH file that does not write out information about its output - /// path. - /// - /// FIXME: This only controls whether \p ORIGINAL_PCH_DIR record is written - /// out or not. Consider either removing that record entirely if it's no - /// longer relevant or switching the default to not write it unless an option - /// is set to true. - unsigned OutputPathIndependentPCM : 1; - /// Output (and read) PCM files regardless of compiler errors. unsigned AllowPCMWithCompilerErrors : 1; @@ -525,8 +516,8 @@ class FrontendOptions { ASTDumpLookups(false), BuildingImplicitModule(false), BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false), IncludeTimestamps(true), UseTemporary(true), -OutputPathIndependentPCM(false), AllowPCMWithCompilerErrors(false), -ModulesShareFileManager(true), TimeTraceGranularity(500) {} +AllowPCMWithCompilerErrors(false), ModulesShareFileManager(true), +TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 58f456486f6de..d48e44982f222 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -343,9 +343,6 @@ enum ControlRecordTypes { /// name. ORIGINAL_FILE, - /// The directory that the PCH was originally created in. - ORIGINAL_PCH_DIR, - /// Record code for file ID of the file or buffer that was used to /// generate the AST file. ORIGINAL_FILE_ID, diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index f4f4f3540b297..530c816fc6c3e 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -455,7 +455,7 @@ class ASTWriter : public ASTDeserializationListener, void WriteBlockInfoBlock(); void WriteControlBlock(Preprocessor , ASTContext , - StringRef isysroot, StringRef OutputFile); + StringRef isysroot); /// Write out the signature and diagnostic options, and return the signature. ASTFileSignature
[clang] 944a86d - [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path
Author: Argyrios Kyrtzidis Date: 2022-07-29T15:21:54-07:00 New Revision: 944a86de7c504121da8958b37b7fe8fff53d43a2 URL: https://github.com/llvm/llvm-project/commit/944a86de7c504121da8958b37b7fe8fff53d43a2 DIFF: https://github.com/llvm/llvm-project/commit/944a86de7c504121da8958b37b7fe8fff53d43a2.diff LOG: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path This is useful to enable sharing of the same PCH file even when it's intended for a different output path. The only information this option disables writing is for `ORIGINAL_PCH_DIR` record which is treated as optional and (when present) used as fallback for resolving input file paths relative to it. Differential Revision: https://reviews.llvm.org/D130710 Added: clang/test/PCH/pch-output-path-independent.c Modified: clang/include/clang/Driver/Options.td clang/include/clang/Frontend/FrontendOptions.h clang/include/clang/Serialization/ASTWriter.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/Serialization/GeneratePCH.cpp Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c723f6164530b..1718234a56988 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6025,6 +6025,9 @@ def fno_validate_pch : Flag<["-"], "fno-validate-pch">, HelpText<"Disable validation of precompiled headers">, MarshallingInfoFlag, "DisableValidationForModuleKind::None">, Normalizer<"makeFlagToValueNormalizer(DisableValidationForModuleKind::All)">; +def fpcm_output_path_independent : Flag<["-"], "fpcm-output-path-independent">, + HelpText<"Output a PCM/PCH file that does not write out information about its output path">, + MarshallingInfoFlag>; def fallow_pcm_with_errors : Flag<["-"], "fallow-pcm-with-compiler-errors">, HelpText<"Accept a PCM file that was created with compiler errors">, MarshallingInfoFlag>; diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index b0e719ffcacf5..0c9c2489e4486 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -344,6 +344,15 @@ class FrontendOptions { /// When using -emit-module, treat the modulemap as a system module. unsigned IsSystemModule : 1; + /// Output a PCM/PCH file that does not write out information about its output + /// path. + /// + /// FIXME: This only controls whether \p ORIGINAL_PCH_DIR record is written + /// out or not. Consider either removing that record entirely if it's no + /// longer relevant or switching the default to not write it unless an option + /// is set to true. + unsigned OutputPathIndependentPCM : 1; + /// Output (and read) PCM files regardless of compiler errors. unsigned AllowPCMWithCompilerErrors : 1; @@ -513,7 +522,8 @@ class FrontendOptions { ASTDumpLookups(false), BuildingImplicitModule(false), BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false), IncludeTimestamps(true), UseTemporary(true), -AllowPCMWithCompilerErrors(false), TimeTraceGranularity(500) {} +OutputPathIndependentPCM(false), AllowPCMWithCompilerErrors(false), +TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 1d4af1b20cdef..f4f4f3540b297 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -573,7 +573,8 @@ class ASTWriter : public ASTDeserializationListener, ASTFileSignature WriteAST(Sema , StringRef OutputFile, Module *WritingModule, StringRef isysroot, bool hasErrors = false, -bool ShouldCacheASTInMemory = false); +bool ShouldCacheASTInMemory = false, +bool OutputPathIndependent = false); /// Emit a token. void AddToken(const Token , RecordDataImpl ); @@ -764,6 +765,7 @@ class PCHGenerator : public SemaConsumer { ASTWriter Writer; bool AllowASTWithErrors; bool ShouldCacheASTInMemory; + bool OutputPathIndependent; protected: ASTWriter () { return Writer; } @@ -776,7 +778,8 @@ class PCHGenerator : public SemaConsumer { std::shared_ptr Buffer, ArrayRef> Extensions, bool AllowASTWithErrors = false, bool IncludeTimestamps = true, - bool ShouldCacheASTInMemory = false); + bool ShouldCacheASTInMemory = false, + bool OutputPathIndependent = false);
[clang] a9ae2f2 - [ASTWriter] Replace `const std::string ` with `StringRef OutputFile` in some of `ASTWriter` functions, NFC
Author: Argyrios Kyrtzidis Date: 2022-07-27T23:02:33-07:00 New Revision: a9ae2f2764364a2c3bfe7d81087b74b465dcb305 URL: https://github.com/llvm/llvm-project/commit/a9ae2f2764364a2c3bfe7d81087b74b465dcb305 DIFF: https://github.com/llvm/llvm-project/commit/a9ae2f2764364a2c3bfe7d81087b74b465dcb305.diff LOG: [ASTWriter] Replace `const std::string ` with `StringRef OutputFile` in some of `ASTWriter` functions, NFC This is to make it consistent with LLVM's string parameter passing convention. Added: Modified: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp Removed: diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h index 83bc7dcdfde3b..1d4af1b20cdef 100644 --- a/clang/include/clang/Serialization/ASTWriter.h +++ b/clang/include/clang/Serialization/ASTWriter.h @@ -455,7 +455,7 @@ class ASTWriter : public ASTDeserializationListener, void WriteBlockInfoBlock(); void WriteControlBlock(Preprocessor , ASTContext , - StringRef isysroot, const std::string ); + StringRef isysroot, StringRef OutputFile); /// Write out the signature and diagnostic options, and return the signature. ASTFileSignature writeUnhashedControlBlock(Preprocessor , @@ -533,8 +533,7 @@ class ASTWriter : public ASTDeserializationListener, void WriteDecl(ASTContext , Decl *D); ASTFileSignature WriteASTCore(Sema , StringRef isysroot, -const std::string , -Module *WritingModule); +StringRef OutputFile, Module *WritingModule); public: /// Create a new precompiled header writer that outputs to @@ -571,7 +570,7 @@ class ASTWriter : public ASTDeserializationListener, /// /// \return the module signature, which eventually will be a hash of /// the module but currently is merely a random 32-bit number. - ASTFileSignature WriteAST(Sema , const std::string , + ASTFileSignature WriteAST(Sema , StringRef OutputFile, Module *WritingModule, StringRef isysroot, bool hasErrors = false, bool ShouldCacheASTInMemory = false); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 0739dcc1ce600..8ca000b340845 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1187,8 +1187,7 @@ ASTFileSignature ASTWriter::writeUnhashedControlBlock(Preprocessor , /// Write the control block. void ASTWriter::WriteControlBlock(Preprocessor , ASTContext , - StringRef isysroot, - const std::string ) { + StringRef isysroot, StringRef OutputFile) { using namespace llvm; Stream.EnterSubblock(CONTROL_BLOCK_ID, 5); @@ -4481,8 +4480,7 @@ time_t ASTWriter::getTimestampForOutput(const FileEntry *E) const { return IncludeTimestamps ? E->getModificationTime() : 0; } -ASTFileSignature ASTWriter::WriteAST(Sema , - const std::string , +ASTFileSignature ASTWriter::WriteAST(Sema , StringRef OutputFile, Module *WritingModule, StringRef isysroot, bool hasErrors, bool ShouldCacheASTInMemory) { @@ -4528,7 +4526,7 @@ static void AddLazyVectorDecls(ASTWriter , Vector , } ASTFileSignature ASTWriter::WriteASTCore(Sema , StringRef isysroot, - const std::string , + StringRef OutputFile, Module *WritingModule) { using namespace llvm; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 8dfaecc - [CGDebugInfo] Access the current working directory from the `VFS`
Author: Argyrios Kyrtzidis Date: 2022-07-26T13:48:39-07:00 New Revision: 8dfaecc4c24494337933aff9d9166486ca0949f1 URL: https://github.com/llvm/llvm-project/commit/8dfaecc4c24494337933aff9d9166486ca0949f1 DIFF: https://github.com/llvm/llvm-project/commit/8dfaecc4c24494337933aff9d9166486ca0949f1.diff LOG: [CGDebugInfo] Access the current working directory from the `VFS` ...instead of calling `llvm::sys::fs::current_path()` directly. Differential Revision: https://reviews.llvm.org/D130443 Added: Modified: clang/include/clang/CodeGen/ModuleBuilder.h clang/lib/CodeGen/CGDebugInfo.cpp clang/lib/CodeGen/CodeGenAction.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/CodeGenModule.h clang/lib/CodeGen/ModuleBuilder.cpp clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp clang/tools/clang-import-test/clang-import-test.cpp clang/unittests/CodeGen/TestCompiler.h clang/unittests/Frontend/CodeGenActionTest.cpp Removed: diff --git a/clang/include/clang/CodeGen/ModuleBuilder.h b/clang/include/clang/CodeGen/ModuleBuilder.h index 26587e73bf6c7..edacd82bf899d 100644 --- a/clang/include/clang/CodeGen/ModuleBuilder.h +++ b/clang/include/clang/CodeGen/ModuleBuilder.h @@ -14,12 +14,17 @@ #define LLVM_CLANG_CODEGEN_MODULEBUILDER_H #include "clang/AST/ASTConsumer.h" +#include "clang/Basic/LLVM.h" namespace llvm { class Constant; class LLVMContext; class Module; class StringRef; + + namespace vfs { + class FileSystem; + } } namespace clang { @@ -98,10 +103,11 @@ class CodeGenerator : public ASTConsumer { /// the allocated CodeGenerator instance. CodeGenerator *CreateLLVMCodeGen(DiagnosticsEngine , llvm::StringRef ModuleName, + IntrusiveRefCntPtr FS, const HeaderSearchOptions , const PreprocessorOptions , const CodeGenOptions , - llvm::LLVMContext& C, + llvm::LLVMContext , CoverageSourceInfo *CoverageInfo = nullptr); } // end namespace clang diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 7e9e86763af99..94c48316add74 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -491,9 +491,11 @@ StringRef CGDebugInfo::getCurrentDirname() { if (!CWDName.empty()) return CWDName; - SmallString<256> CWD; - llvm::sys::fs::current_path(CWD); - return CWDName = internString(CWD); + llvm::ErrorOr CWD = + CGM.getFileSystem()->getCurrentWorkingDirectory(); + if (!CWD) +return StringRef(); + return CWDName = internString(*CWD); } void CGDebugInfo::CreateCompileUnit() { diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index 4ffbecdf27411..12c6b3f49c432 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/clang/lib/CodeGen/CodeGenAction.cpp @@ -146,6 +146,7 @@ namespace clang { public: BackendConsumer(BackendAction Action, DiagnosticsEngine , +IntrusiveRefCntPtr FS, const HeaderSearchOptions , const PreprocessorOptions , const CodeGenOptions , @@ -159,8 +160,8 @@ namespace clang { AsmOutStream(std::move(OS)), Context(nullptr), LLVMIRGeneration("irgen", "LLVM IR Generation Time"), LLVMIRGenerationRefCount(0), - Gen(CreateLLVMCodeGen(Diags, InFile, HeaderSearchOpts, PPOpts, -CodeGenOpts, C, CoverageInfo)), + Gen(CreateLLVMCodeGen(Diags, InFile, std::move(FS), HeaderSearchOpts, +PPOpts, CodeGenOpts, C, CoverageInfo)), LinkModules(std::move(LinkModules)) { TimerIsEnabled = CodeGenOpts.TimePasses; llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses; @@ -171,6 +172,7 @@ namespace clang { // to use the clang diagnostic handler for IR input files. It avoids // initializing the OS field. BackendConsumer(BackendAction Action, DiagnosticsEngine , +IntrusiveRefCntPtr FS, const HeaderSearchOptions , const PreprocessorOptions , const CodeGenOptions , @@ -183,8 +185,8 @@ namespace clang { Context(nullptr), LLVMIRGeneration("irgen", "LLVM IR Generation Time"), LLVMIRGenerationRefCount(0), - Gen(CreateLLVMCodeGen(Diags, "", HeaderSearchOpts, PPOpts, -CodeGenOpts, C, CoverageInfo)), + Gen(CreateLLVMCodeGen(Diags, "", std::move(FS), HeaderSearchOpts, +PPOpts, CodeGenOpts, C, CoverageInfo)), LinkModules(std::move(LinkModules)),
[clang] d1b58ca - [unittests/Tooling/DependencyScannerTest] Add a target triple for `ScanDepsWithFS` test
Author: Argyrios Kyrtzidis Date: 2022-07-18T16:55:07-07:00 New Revision: d1b58cada61aa8bc44d8e8ef9c23ed12ef7b549b URL: https://github.com/llvm/llvm-project/commit/d1b58cada61aa8bc44d8e8ef9c23ed12ef7b549b DIFF: https://github.com/llvm/llvm-project/commit/d1b58cada61aa8bc44d8e8ef9c23ed12ef7b549b.diff LOG: [unittests/Tooling/DependencyScannerTest] Add a target triple for `ScanDepsWithFS` test This should fix the `clang-ppc64-aix` builder. Added: Modified: clang/unittests/Tooling/DependencyScannerTest.cpp Removed: diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp index 0e2498097e41b..abcc2c787b0d0 100644 --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -207,7 +207,11 @@ TEST(DependencyScanner, ScanDepsReuseFilemanagerHasInclude) { } TEST(DependencyScanner, ScanDepsWithFS) { - std::vector CommandLine = {"clang", "-c", "test.cpp", + std::vector CommandLine = {"clang", + "-target", + "x86_64-apple-macosx10.7", + "-c", + "test.cpp", "-o" "test.cpp.o"}; StringRef CWD = "/root"; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fbbabd4 - [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`
Author: Argyrios Kyrtzidis Date: 2022-07-18T09:37:17-07:00 New Revision: fbbabd4ca06a030b714220f17b1c6e74a9ea3c4d URL: https://github.com/llvm/llvm-project/commit/fbbabd4ca06a030b714220f17b1c6e74a9ea3c4d DIFF: https://github.com/llvm/llvm-project/commit/fbbabd4ca06a030b714220f17b1c6e74a9ea3c4d.diff LOG: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool` Also include a unit test to validate that the `vfs::FileSystem` object is properly used. Differential Revision: https://reviews.llvm.org/D129912 Added: Modified: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp clang/unittests/Tooling/DependencyScannerTest.cpp Removed: diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h index a85d333ba6b18..209cc81e38dd8 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h @@ -68,7 +68,9 @@ struct FullDependenciesResult { class DependencyScanningTool { public: /// Construct a dependency scanning tool. - DependencyScanningTool(DependencyScanningService ); + DependencyScanningTool(DependencyScanningService , + llvm::IntrusiveRefCntPtr FS = + llvm::vfs::createPhysicalFileSystem()); /// Print out the dependency information into a string using the dependency /// file format that is specified in the options (-MD is the default) and diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h index 337bba2e72da9..d6c0f2f1c6d69 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h @@ -52,7 +52,8 @@ class DependencyConsumer { /// using the regular processing run. class DependencyScanningWorker { public: - DependencyScanningWorker(DependencyScanningService ); + DependencyScanningWorker(DependencyScanningService , + llvm::IntrusiveRefCntPtr FS); /// Run the dependency scanning tool for a given clang driver command-line, /// and report the discovered dependencies to the provided consumer. If \p diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp index 43127ea2df983..411fd9676ffdb 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -50,8 +50,9 @@ FullDependencies::getCommandLineWithoutModulePaths() const { } DependencyScanningTool::DependencyScanningTool( -DependencyScanningService ) -: Worker(Service) {} +DependencyScanningService , +llvm::IntrusiveRefCntPtr FS) +: Worker(Service, std::move(FS)) {} llvm::Expected DependencyScanningTool::getDependencyFile( const std::vector , StringRef CWD, diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index e7d1375c83f0b..474808d888ec2 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -264,7 +264,8 @@ class DependencyScanningAction : public tooling::ToolAction { } // end anonymous namespace DependencyScanningWorker::DependencyScanningWorker( -DependencyScanningService ) +DependencyScanningService , +llvm::IntrusiveRefCntPtr FS) : Format(Service.getFormat()), OptimizeArgs(Service.canOptimizeArgs()) { PCHContainerOps = std::make_shared(); PCHContainerOps->registerReader( @@ -274,8 +275,8 @@ DependencyScanningWorker::DependencyScanningWorker( PCHContainerOps->registerWriter( std::make_unique()); - auto OverlayFS = llvm::makeIntrusiveRefCnt( - llvm::vfs::createPhysicalFileSystem()); + auto OverlayFS = + llvm::makeIntrusiveRefCnt(std::move(FS)); InMemoryFS = llvm::makeIntrusiveRefCnt(); OverlayFS->pushOverlay(InMemoryFS); RealFS = OverlayFS; diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp index 6480ceacee963..0e2498097e41b 100644 --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -14,19 +14,21 @@ #include "clang/Frontend/FrontendAction.h" #include
[clang] 46a6989 - [unittests/Tooling/DependencyScannerTest.cpp] Use `using namespace` instead of wrapping the `.cpp` file contents in namespaces, NFC
Author: Argyrios Kyrtzidis Date: 2022-07-15T16:10:36-07:00 New Revision: 46a69897364354c9ffcfb1f5f2341e675898d116 URL: https://github.com/llvm/llvm-project/commit/46a69897364354c9ffcfb1f5f2341e675898d116 DIFF: https://github.com/llvm/llvm-project/commit/46a69897364354c9ffcfb1f5f2341e675898d116.diff LOG: [unittests/Tooling/DependencyScannerTest.cpp] Use `using namespace` instead of wrapping the `.cpp` file contents in namespaces, NFC This makes the file consistent with the coding style of the rest of LLVM. Added: Modified: clang/unittests/Tooling/DependencyScannerTest.cpp Removed: diff --git a/clang/unittests/Tooling/DependencyScannerTest.cpp b/clang/unittests/Tooling/DependencyScannerTest.cpp index a9d6e6e7fb6fd..6480ceacee963 100644 --- a/clang/unittests/Tooling/DependencyScannerTest.cpp +++ b/clang/unittests/Tooling/DependencyScannerTest.cpp @@ -25,8 +25,8 @@ #include #include -namespace clang { -namespace tooling { +using namespace clang; +using namespace tooling; namespace { @@ -203,6 +203,3 @@ TEST(DependencyScanner, ScanDepsReuseFilemanagerHasInclude) { EXPECT_EQ(convert_to_slash(Deps[4]), "/root/header.h"); EXPECT_EQ(convert_to_slash(Deps[5]), "/root/symlink.h"); } - -} // end namespace tooling -} // end namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fe3780f - [DependencyScanningTool.cpp] Use `using namespace` instead of wrapping the `.cpp` file contents in namespaces, NFC
Author: Argyrios Kyrtzidis Date: 2022-07-11T17:44:17-07:00 New Revision: fe3780f32ae81187e0700e52bd551cc02c7a63b4 URL: https://github.com/llvm/llvm-project/commit/fe3780f32ae81187e0700e52bd551cc02c7a63b4 DIFF: https://github.com/llvm/llvm-project/commit/fe3780f32ae81187e0700e52bd551cc02c7a63b4.diff LOG: [DependencyScanningTool.cpp] Use `using namespace` instead of wrapping the `.cpp` file contents in namespaces, NFC This makes the file consistent with the coding style of the rest of LLVM. Added: Modified: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp Removed: diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp index 26c2b2b2f394..575aa46bff1b 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -9,9 +9,9 @@ #include "clang/Tooling/DependencyScanning/DependencyScanningTool.h" #include "clang/Frontend/Utils.h" -namespace clang { -namespace tooling { -namespace dependencies { +using namespace clang; +using namespace tooling; +using namespace dependencies; std::vector FullDependencies::getCommandLine( std::function LookupPCMPath) const { @@ -192,7 +192,3 @@ DependencyScanningTool::getFullDependencies( return std::move(Result); return Consumer.getFullDependencies(CommandLine); } - -} // end namespace dependencies -} // end namespace tooling -} // end namespace clang ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 93d6fdf - [Driver] Ignore the clang modules validation-related flags if clang modules are not enabled
Author: Argyrios Kyrtzidis Date: 2022-07-03T21:26:15-07:00 New Revision: 93d6fdfc232c59975d52146532693178def5ad16 URL: https://github.com/llvm/llvm-project/commit/93d6fdfc232c59975d52146532693178def5ad16 DIFF: https://github.com/llvm/llvm-project/commit/93d6fdfc232c59975d52146532693178def5ad16.diff LOG: [Driver] Ignore the clang modules validation-related flags if clang modules are not enabled If clang modules are not enabled it becomes unnecessary to read the session timestamp file in order to pass `-fbuild-session-timestamp` to the `cc1` invocation. Differential Revision: https://reviews.llvm.org/D129030 Added: Modified: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/modules.m Removed: diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index c9bbdb2ac72e3..bc437d66a03ce 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3750,38 +3750,49 @@ static void RenderModulesOptions(Compilation , const Driver , Args.AddLastArg(CmdArgs, options::OPT_fmodules_prune_interval); Args.AddLastArg(CmdArgs, options::OPT_fmodules_prune_after); - Args.AddLastArg(CmdArgs, options::OPT_fbuild_session_timestamp); + if (HaveClangModules) { +Args.AddLastArg(CmdArgs, options::OPT_fbuild_session_timestamp); - if (Arg *A = Args.getLastArg(options::OPT_fbuild_session_file)) { -if (Args.hasArg(options::OPT_fbuild_session_timestamp)) - D.Diag(diag::err_drv_argument_not_allowed_with) - << A->getAsString(Args) << "-fbuild-session-timestamp"; +if (Arg *A = Args.getLastArg(options::OPT_fbuild_session_file)) { + if (Args.hasArg(options::OPT_fbuild_session_timestamp)) +D.Diag(diag::err_drv_argument_not_allowed_with) +<< A->getAsString(Args) << "-fbuild-session-timestamp"; -llvm::sys::fs::file_status Status; -if (llvm::sys::fs::status(A->getValue(), Status)) - D.Diag(diag::err_drv_no_such_file) << A->getValue(); -CmdArgs.push_back(Args.MakeArgString( -"-fbuild-session-timestamp=" + -Twine((uint64_t)std::chrono::duration_cast( - Status.getLastModificationTime().time_since_epoch()) - .count(; - } + llvm::sys::fs::file_status Status; + if (llvm::sys::fs::status(A->getValue(), Status)) +D.Diag(diag::err_drv_no_such_file) << A->getValue(); + CmdArgs.push_back(Args.MakeArgString( + "-fbuild-session-timestamp=" + + Twine((uint64_t)std::chrono::duration_cast( +Status.getLastModificationTime().time_since_epoch()) +.count(; +} - if (Args.getLastArg(options::OPT_fmodules_validate_once_per_build_session)) { -if (!Args.getLastArg(options::OPT_fbuild_session_timestamp, - options::OPT_fbuild_session_file)) - D.Diag(diag::err_drv_modules_validate_once_requires_timestamp); +if (Args.getLastArg( +options::OPT_fmodules_validate_once_per_build_session)) { + if (!Args.getLastArg(options::OPT_fbuild_session_timestamp, + options::OPT_fbuild_session_file)) +D.Diag(diag::err_drv_modules_validate_once_requires_timestamp); -Args.AddLastArg(CmdArgs, -options::OPT_fmodules_validate_once_per_build_session); - } + Args.AddLastArg(CmdArgs, + options::OPT_fmodules_validate_once_per_build_session); +} - if (Args.hasFlag(options::OPT_fmodules_validate_system_headers, - options::OPT_fno_modules_validate_system_headers, - ImplicitModules)) -CmdArgs.push_back("-fmodules-validate-system-headers"); +if (Args.hasFlag(options::OPT_fmodules_validate_system_headers, + options::OPT_fno_modules_validate_system_headers, + ImplicitModules)) + CmdArgs.push_back("-fmodules-validate-system-headers"); - Args.AddLastArg(CmdArgs, options::OPT_fmodules_disable_diagnostic_validation); +Args.AddLastArg(CmdArgs, +options::OPT_fmodules_disable_diagnostic_validation); + } else { +Args.ClaimAllArgs(options::OPT_fbuild_session_timestamp); +Args.ClaimAllArgs(options::OPT_fbuild_session_file); +Args.ClaimAllArgs(options::OPT_fmodules_validate_once_per_build_session); +Args.ClaimAllArgs(options::OPT_fmodules_validate_system_headers); +Args.ClaimAllArgs(options::OPT_fno_modules_validate_system_headers); +Args.ClaimAllArgs(options::OPT_fmodules_disable_diagnostic_validation); + } } static void RenderCharacterOptions(const ArgList , const llvm::Triple , diff --git a/clang/test/Driver/modules.m b/clang/test/Driver/modules.m index 67140926f176b..4b67f7c7bec4f 100644 --- a/clang/test/Driver/modules.m +++ b/clang/test/Driver/modules.m @@ -4,27 +4,27 @@ // RUN: %clang -fmodules
[clang-tools-extra] 0d3a2b4 - [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback
Author: Argyrios Kyrtzidis Date: 2022-07-01T14:22:31-07:00 New Revision: 0d3a2b4c6601d4ff341119aa537db184197d83de URL: https://github.com/llvm/llvm-project/commit/0d3a2b4c6601d4ff341119aa537db184197d83de DIFF: https://github.com/llvm/llvm-project/commit/0d3a2b4c6601d4ff341119aa537db184197d83de.diff LOG: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback This is a preprocessor callback focused on the lexed file changing, without conflating effects of line number directives and other pragmas. A client that only cares about what files the lexer processes, like dependency generation, can use this more straightforward callback instead of `PPCallbacks::FileChanged()`. Clients that want the pragma directive effects as well can keep using `FileChanged()`. A use case where `PPCallbacks::LexedFileChanged()` is particularly simpler to use than `FileChanged()` is in a situation where a client wants to keep track of lexed file changes that include changes from/to the predefines buffer, where it becomes unnecessary complicated trying to use `FileChanged()` while filtering out the pragma directives effects callbacks. Also take the opportunity to provide information about the prior `FileID` the `Lexer` moved from, even when entering a new file. Differential Revision: https://reviews.llvm.org/D128947 Added: Modified: clang-tools-extra/test/pp-trace/pp-trace-include.cpp clang/include/clang/Lex/PPCallbacks.h clang/lib/Frontend/DependencyFile.cpp clang/lib/Lex/PPLexerChange.cpp Removed: diff --git a/clang-tools-extra/test/pp-trace/pp-trace-include.cpp b/clang-tools-extra/test/pp-trace/pp-trace-include.cpp index 96b4014025b78..db0b2c89430a2 100644 --- a/clang-tools-extra/test/pp-trace/pp-trace-include.cpp +++ b/clang-tools-extra/test/pp-trace/pp-trace-include.cpp @@ -13,7 +13,7 @@ // CHECK-NEXT: Loc: ":1:1" // CHECK-NEXT: Reason: EnterFile // CHECK-NEXT: FileType: C_User -// CHECK-NEXT: PrevFID: (invalid) +// CHECK-NEXT: PrevFID: "{{.*}}{{[/\\]}}pp-trace-include.cpp" // CHECK-NEXT: - Callback: FileChanged // CHECK-NEXT: Loc: ":1:1" // CHECK-NEXT: Reason: RenameFile @@ -64,7 +64,7 @@ // CHECK-NEXT: Loc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1" // CHECK-NEXT: Reason: EnterFile // CHECK-NEXT: FileType: C_User -// CHECK-NEXT: PrevFID: (invalid) +// CHECK-NEXT: PrevFID: "{{.*}}{{[/\\]}}pp-trace-include.cpp" // CHECK-NEXT: - Callback: InclusionDirective // CHECK-NEXT: HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1" // CHECK-NEXT: IncludeTok: include @@ -79,7 +79,7 @@ // CHECK-NEXT: Loc: "{{.*}}{{[/\\]}}Inputs/Level2A.h:1:1" // CHECK-NEXT: Reason: EnterFile // CHECK-NEXT: FileType: C_User -// CHECK-NEXT: PrevFID: (invalid) +// CHECK-NEXT: PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h" // CHECK-NEXT: - Callback: MacroDefined // CHECK-NEXT: MacroNameTok: MACRO_2A // CHECK-NEXT: MacroDirective: MD_Define @@ -110,7 +110,7 @@ // CHECK-NEXT: Loc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1" // CHECK-NEXT: Reason: EnterFile // CHECK-NEXT: FileType: C_User -// CHECK-NEXT: PrevFID: (invalid) +// CHECK-NEXT: PrevFID: "{{.*}}{{[/\\]}}pp-trace-include.cpp" // CHECK-NEXT: - Callback: InclusionDirective // CHECK-NEXT: HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1" // CHECK-NEXT: IncludeTok: include @@ -125,7 +125,7 @@ // CHECK-NEXT: Loc: "{{.*}}{{[/\\]}}Inputs/Level2B.h:1:1" // CHECK-NEXT: Reason: EnterFile // CHECK-NEXT: FileType: C_User -// CHECK-NEXT: PrevFID: (invalid) +// CHECK-NEXT: PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1B.h" // CHECK-NEXT: - Callback: MacroDefined // CHECK-NEXT: MacroNameTok: MACRO_2B // CHECK-NEXT: MacroDirective: MD_Define diff --git a/clang/include/clang/Lex/PPCallbacks.h b/clang/include/clang/Lex/PPCallbacks.h index 5f7cfab00e41a..045df8711a41b 100644 --- a/clang/include/clang/Lex/PPCallbacks.h +++ b/clang/include/clang/Lex/PPCallbacks.h @@ -43,12 +43,34 @@ class PPCallbacks { /// Callback invoked whenever a source file is entered or exited. /// /// \param Loc Indicates the new location. - /// \param PrevFID the file that was exited if \p Reason is ExitFile. + /// \param PrevFID the file that was exited if \p Reason is ExitFile or the + /// the file before the new one entered for \p Reason EnterFile. virtual void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, FileID PrevFID = FileID()) { } + enum class LexedFileChangeReason { EnterFile, ExitFile }; + + /// Callback invoked whenever the \p Lexer moves to a diff erent file for + /// lexing. Unlike \p FileChanged line number directives and other related + /// pragmas do not trigger callbacks to \p LexedFileChanged. + /// + /// \param FID The \p FileID that the \p Lexer moved to. + /// + /// \param Reason
[clang] c68b8c8 - [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing
Author: Argyrios Kyrtzidis Date: 2022-06-29T15:50:16-07:00 New Revision: c68b8c84eb17e4c125897a8a381aa31eea5e5c58 URL: https://github.com/llvm/llvm-project/commit/c68b8c84eb17e4c125897a8a381aa31eea5e5c58 DIFF: https://github.com/llvm/llvm-project/commit/c68b8c84eb17e4c125897a8a381aa31eea5e5c58.diff LOG: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing Otherwise a header may be erroneously marked as having a header macro guard and won't get re-included. Differential Revision: https://reviews.llvm.org/D128772 Added: clang/test/ClangScanDeps/more-content-after-headerguard.c Modified: clang/lib/Lex/Lexer.cpp Removed: diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp index 122861c4d2520..6820057642bea 100644 --- a/clang/lib/Lex/Lexer.cpp +++ b/clang/lib/Lex/Lexer.cpp @@ -4248,6 +4248,10 @@ bool Lexer::LexDependencyDirectiveToken(Token ) { const dependency_directives_scan::Token = DepDirectives.front().Tokens[NextDepDirectiveTokenIndex++]; + if (NextDepDirectiveTokenIndex > 1 || DDTok.Kind != tok::hash) { +// Read something other than a preprocessor directive hash. +MIOpt.ReadToken(); + } const char *TokPtr = convertDependencyDirectiveToken(DDTok, Result); diff --git a/clang/test/ClangScanDeps/more-content-after-headerguard.c b/clang/test/ClangScanDeps/more-content-after-headerguard.c new file mode 100644 index 0..5c47508e03ab3 --- /dev/null +++ b/clang/test/ClangScanDeps/more-content-after-headerguard.c @@ -0,0 +1,47 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json + +// RUN: clang-scan-deps -compilation-database %t/cdb.json | FileCheck %s + +// CHECK: t.c +// CHECK: top.h +// CHECK: n1.h +// CHECK: n2.h +// CHECK: n3.h + +//--- cdb.json.template +[ + { +"directory": "DIR", +"command": "clang -fsyntax-only DIR/t.c", +"file": "DIR/t.c" + } +] + +//--- t.c + +#include "top.h" +#define INCLUDE_N3 +#include "top.h" + +//--- top.h +#ifndef _TOP_H_ +#define _TOP_H_ + +#include "n1.h" + +#endif + +// More stuff after following '#endif', should invalidate the macro guard optimization, +// and allow `top.h` to get re-included. +#include "n2.h" + +//--- n1.h + +//--- n2.h +#ifdef INCLUDE_N3 +#include "n3.h" +#endif + +//--- n3.h ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] f7e19a5 - [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited
Author: Argyrios Kyrtzidis Date: 2022-06-13T21:46:46-07:00 New Revision: f7e19a59284208712314a2d0702b48c445909130 URL: https://github.com/llvm/llvm-project/commit/f7e19a59284208712314a2d0702b48c445909130 DIFF: https://github.com/llvm/llvm-project/commit/f7e19a59284208712314a2d0702b48c445909130.diff LOG: [Lex] Keep track of skipped preprocessor blocks and advance the lexer directly if they are revisited This speeds up preprocessing, specifically for preprocessing the clang sources time is reduced by about -36%, using measurements on M1Pro with a release+thinLTO build. Differential Revision: https://reviews.llvm.org/D127379 Added: Modified: clang/include/clang/Lex/Preprocessor.h clang/lib/Lex/PPDirectives.cpp Removed: diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 81d1481e88fa8..9bd4b40a60770 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -952,6 +952,18 @@ class Preprocessor { /// of that list. MacroInfoChain *MIChainHead = nullptr; + /// True if \p Preprocessor::SkipExcludedConditionalBlock() is running. + /// This is used to guard against calling this function recursively. + /// + /// See comments at the use-site for more context about why it is needed. + bool SkippingExcludedConditionalBlock = false; + + /// Keeps track of skipped range mappings that were recorded while skipping + /// excluded conditional directives. It maps the source buffer pointer at + /// the beginning of a skipped block, to the number of bytes that should be + /// skipped. + llvm::DenseMap RecordedSkippedRanges; + void updateOutOfDateIdentifier(IdentifierInfo ) const; public: diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 1356dc097dfcc..70d001fbaec77 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -33,15 +33,16 @@ #include "clang/Lex/Token.h" #include "clang/Lex/VariadicMacroSupport.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/StringSwitch.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSwitch.h" #include "llvm/Support/AlignOf.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Path.h" +#include "llvm/Support/SaveAndRestore.h" #include #include #include @@ -481,6 +482,19 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, bool FoundNonSkipPortion, bool FoundElse, SourceLocation ElseLoc) { + // In SkippingRangeStateTy we are depending on SkipExcludedConditionalBlock() + // not getting called recursively by storing the RecordedSkippedRanges + // DenseMap lookup pointer (field SkipRangePtr). SkippingRangeStateTy expects + // that RecordedSkippedRanges won't get modified and SkipRangePtr won't be + // invalidated. If this changes and there is a need to call + // SkipExcludedConditionalBlock() recursively, SkippingRangeStateTy should + // change to do a second lookup in endLexPass function instead of reusing the + // lookup pointer. + assert(!SkippingExcludedConditionalBlock && + "calling SkipExcludedConditionalBlock recursively"); + llvm::SaveAndRestore SARSkipping(SkippingExcludedConditionalBlock, + true); + ++NumSkipped; assert(!CurTokenLexer && CurPPLexer && "Lexing a macro, not a file?"); @@ -495,10 +509,53 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, CurPPLexer->LexingRawMode = true; Token Tok; SourceLocation endLoc; + + /// Keeps track and caches skipped ranges and also retrieves a prior skipped + /// range if the same block is re-visited. + struct SkippingRangeStateTy { +Preprocessor + +const char *BeginPtr = nullptr; +unsigned *SkipRangePtr = nullptr; + +SkippingRangeStateTy(Preprocessor ) : PP(PP) {} + +void beginLexPass() { + if (BeginPtr) +return; // continue skipping a block. + + // Initiate a skipping block and adjust the lexer if we already skipped it + // before. + BeginPtr = PP.CurLexer->getBufferLocation(); + SkipRangePtr = [BeginPtr]; + if (*SkipRangePtr) { +PP.CurLexer->seek(PP.CurLexer->getCurrentBufferOffset() + *SkipRangePtr, + /*IsAtStartOfLine*/ true); + } +} + +void endLexPass(const char *Hashptr) { + if (!BeginPtr) { +// Not doing normal lexing. +assert(PP.CurLexer->isDependencyDirectivesLexer()); +return; + } + + // Finished skipping a block, record the
[clang] fbaa8b9 - [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks
Author: Argyrios Kyrtzidis Date: 2022-06-10T13:32:19-07:00 New Revision: fbaa8b9ae5f3c6637be7d4dae6adaab4be811625 URL: https://github.com/llvm/llvm-project/commit/fbaa8b9ae5f3c6637be7d4dae6adaab4be811625 DIFF: https://github.com/llvm/llvm-project/commit/fbaa8b9ae5f3c6637be7d4dae6adaab4be811625.diff LOG: [Lex] Fix `fixits` for typo-corrections of preprocessing directives within skipped blocks The `EndLoc` parameter was always unset so no fixit was emitted. But it is also unnecessary for determining the range so we can remove it. Differential Revision: https://reviews.llvm.org/D127251 Added: Modified: clang/include/clang/Lex/Preprocessor.h clang/lib/Lex/PPDirectives.cpp clang/test/Preprocessor/suggest-typoed-directive.c Removed: diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index 6d8f03e3ceb1b..81d1481e88fa8 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -2243,10 +2243,7 @@ class Preprocessor { /// /// \param Tok - Token that represents the directive /// \param Directive - String reference for the directive name - /// \param EndLoc - End location for fixit - void SuggestTypoedDirective(const Token , - StringRef Directive, - const SourceLocation ) const; + void SuggestTypoedDirective(const Token , StringRef Directive) const; /// We just read a \#if or related directive and decided that the /// subsequent tokens are in the \#if'd out portion of the diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 97d7466d79a19..1356dc097dfcc 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -444,8 +444,7 @@ SourceLocation Preprocessor::CheckEndOfDirective(const char *DirType, } void Preprocessor::SuggestTypoedDirective(const Token , - StringRef Directive, - const SourceLocation ) const { + StringRef Directive) const { // If this is a `.S` file, treat unknown # directives as non-preprocessor // directives. if (getLangOpts().AsmPreprocessor) return; @@ -457,11 +456,14 @@ void Preprocessor::SuggestTypoedDirective(const Token , Candidates.insert(Candidates.end(), {"elifdef", "elifndef"}); if (Optional Sugg = findSimilarStr(Directive, Candidates)) { -CharSourceRange DirectiveRange = -CharSourceRange::getCharRange(Tok.getLocation(), EndLoc); -std::string SuggValue = Sugg.getValue().str(); - -auto Hint = FixItHint::CreateReplacement(DirectiveRange, "#" + SuggValue); +// Directive cannot be coming from macro. +assert(Tok.getLocation().isFileID()); +CharSourceRange DirectiveRange = CharSourceRange::getCharRange( +Tok.getLocation(), +Tok.getLocation().getLocWithOffset(Directive.size())); +StringRef SuggValue = Sugg.getValue(); + +auto Hint = FixItHint::CreateReplacement(DirectiveRange, SuggValue); Diag(Tok, diag::warn_pp_invalid_directive) << 1 << SuggValue << Hint; } } @@ -596,7 +598,7 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, /*foundnonskip*/false, /*foundelse*/false); } else { -SuggestTypoedDirective(Tok, Directive, endLoc); +SuggestTypoedDirective(Tok, Directive); } } else if (Directive[0] == 'e') { StringRef Sub = Directive.substr(1); @@ -758,10 +760,10 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc, } } } else { -SuggestTypoedDirective(Tok, Directive, endLoc); +SuggestTypoedDirective(Tok, Directive); } } else { - SuggestTypoedDirective(Tok, Directive, endLoc); + SuggestTypoedDirective(Tok, Directive); } CurPPLexer->ParsingPreprocessorDirective = false; diff --git a/clang/test/Preprocessor/suggest-typoed-directive.c b/clang/test/Preprocessor/suggest-typoed-directive.c index 8dd7ef4feaf69..cc15efee09e1c 100644 --- a/clang/test/Preprocessor/suggest-typoed-directive.c +++ b/clang/test/Preprocessor/suggest-typoed-directive.c @@ -1,6 +1,7 @@ // RUN: %clang_cc1 -fsyntax-only -verify=pre-c2x-cpp2b %s // RUN: %clang_cc1 -std=c2x -fsyntax-only -verify=c2x-cpp2b %s // RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only -verify=c2x-cpp2b %s +// RUN: %clang_cc1 -x c++ -std=c++2b -fsyntax-only %s -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s // id:pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}} // ifd: pre-c2x-cpp2b-warning@+12 {{invalid preprocessing directive, did you mean '#if'?}} @@ -17,8 +18,8 @@ #id #ifd #ifde -#elf -#elsif +# elf +#
[clang] fad6e37 - [Lex] Fix crash during dependency scanning while skipping an unmatched `#if`
Author: Argyrios Kyrtzidis Date: 2022-05-27T23:59:30-07:00 New Revision: fad6e37995b461a7750bdc203aad37eca9532fd5 URL: https://github.com/llvm/llvm-project/commit/fad6e37995b461a7750bdc203aad37eca9532fd5 DIFF: https://github.com/llvm/llvm-project/commit/fad6e37995b461a7750bdc203aad37eca9532fd5.diff LOG: [Lex] Fix crash during dependency scanning while skipping an unmatched `#if` Added: clang/test/ClangScanDeps/skipping-unmatched-if.c Modified: clang/lib/Lex/Lexer.cpp Removed: diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp index a0a7a6ae789b..d4601261b58b 100644 --- a/clang/lib/Lex/Lexer.cpp +++ b/clang/lib/Lex/Lexer.cpp @@ -4223,6 +4223,7 @@ bool Lexer::LexDependencyDirectiveTokenWhileSkipping(Token ) { } break; case pp_eof: + NextDepDirectiveTokenIndex = 0; return LexEndOfFile(Result, BufferEnd); } } while (!Stop); diff --git a/clang/test/ClangScanDeps/skipping-unmatched-if.c b/clang/test/ClangScanDeps/skipping-unmatched-if.c new file mode 100644 index ..fec7857d6bfd --- /dev/null +++ b/clang/test/ClangScanDeps/skipping-unmatched-if.c @@ -0,0 +1,27 @@ +// Check dependency scanning when skipping an unmatched #if + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json + +// RUN: not clang-scan-deps -compilation-database %t/cdb.json 2>&1 | FileCheck %s +// CHECK: header1.h:1:2: error: unterminated conditional directive + +//--- cdb.json.template +[{ + "directory" : "DIR", + "command" : "clang -target x86_64-apple-macosx10.7 -c DIR/test.cpp -o DIR/test.o", + "file" : "DIR/test.o" +}] + +//--- test.cpp +#include "header1.h" +#include "header2.h" + +//--- header1.h +#if 0 + +//--- header2.h +#ifndef _HEADER2_H_ +#define _HEADER2_H_ +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] edcd06b - [test/ClangScanDeps] Add a target triple for `macro-expansions.cpp`
Author: Argyrios Kyrtzidis Date: 2022-05-26T17:18:32-07:00 New Revision: edcd06ba8b4123af83274845316b037b3864f8cb URL: https://github.com/llvm/llvm-project/commit/edcd06ba8b4123af83274845316b037b3864f8cb DIFF: https://github.com/llvm/llvm-project/commit/edcd06ba8b4123af83274845316b037b3864f8cb.diff LOG: [test/ClangScanDeps] Add a target triple for `macro-expansions.cpp` This should fix the `clang-ppc64-aix` builder. Added: Modified: clang/test/ClangScanDeps/macro-expansions.cpp Removed: diff --git a/clang/test/ClangScanDeps/macro-expansions.cpp b/clang/test/ClangScanDeps/macro-expansions.cpp index 78415d6716c8..c8c21f87e600 100644 --- a/clang/test/ClangScanDeps/macro-expansions.cpp +++ b/clang/test/ClangScanDeps/macro-expansions.cpp @@ -15,7 +15,7 @@ //--- cdb.json.template [{ "directory" : "DIR", - "command" : "clang -c DIR/test.cpp -o DIR/test.o", + "command" : "clang -target x86_64-apple-macosx10.7 -c DIR/test.cpp -o DIR/test.o", "file" : "DIR/test.o" }] ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] b4c83a1 - [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources
Author: Argyrios Kyrtzidis Date: 2022-05-26T12:50:06-07:00 New Revision: b4c83a13f664582015ea22924b9a0c6290d41f5b URL: https://github.com/llvm/llvm-project/commit/b4c83a13f664582015ea22924b9a0c6290d41f5b DIFF: https://github.com/llvm/llvm-project/commit/b4c83a13f664582015ea22924b9a0c6290d41f5b.diff LOG: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources This is a commit with the following changes: * Remove `ExcludedPreprocessorDirectiveSkipMapping` and related functionality Removes `ExcludedPreprocessorDirectiveSkipMapping`; its intended benefit for fast skipping of excluded directived blocks will be superseded by a follow-up patch in the series that will use dependency scanning lexing for the same purpose. * Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources Replaces the "source minimization" mechanism with a mechanism that produces lexed dependency directives tokens. * Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer` This is bringing the following benefits: * Full access to the preprocessor state during dependency scanning. E.g. a component can see what includes were taken and where they were located in the actual sources. * Improved performance for dependency scanning. Measurements with a release+thin-LTO build shows ~ -11% reduction in wall time. * Opportunity to use dependency scanning lexing to speed-up skipping of excluded conditional blocks during normal preprocessing (as follow-up, not part of this patch). For normal preprocessing measurements show differences are below the noise level. Since, after this change, we don't minimize sources and pass them in place of the real sources, `DependencyScanningFilesystem` is not technically necessary, but it has valuable performance benefits for caching file `stat`s along with the results of scanning the sources. So the setup of using the `DependencyScanningFilesystem` during a dependency scan remains. Differential Revision: https://reviews.llvm.org/D125486 Differential Revision: https://reviews.llvm.org/D125487 Differential Revision: https://reviews.llvm.org/D125488 Added: Modified: clang/include/clang/Lex/DependencyDirectivesScanner.h clang/include/clang/Lex/Lexer.h clang/include/clang/Lex/Preprocessor.h clang/include/clang/Lex/PreprocessorOptions.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Lex/DependencyDirectivesScanner.cpp clang/lib/Lex/Lexer.cpp clang/lib/Lex/PPDirectives.cpp clang/lib/Lex/PPLexerChange.cpp clang/lib/Lex/Preprocessor.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp clang/test/Lexer/minimize_source_to_dependency_directives_invalid_macro_name.c clang/test/Lexer/minimize_source_to_dependency_directives_pragmas.c clang/unittests/Lex/DependencyDirectivesScannerTest.cpp clang/unittests/Tooling/DependencyScannerTest.cpp Removed: clang/include/clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h diff --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h b/clang/include/clang/Lex/DependencyDirectivesScanner.h index b65891c6b8aba..1ea7e79a0d682 100644 --- a/clang/include/clang/Lex/DependencyDirectivesScanner.h +++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h @@ -19,15 +19,41 @@ #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringRef.h" namespace clang { +namespace tok { +enum TokenKind : unsigned short; +} + class DiagnosticsEngine; namespace dependency_directives_scan { +/// Token lexed as part of dependency directive scanning. +struct Token { + /// Offset into the original source input. + unsigned Offset; + unsigned Length; + tok::TokenKind Kind; + unsigned short Flags; + + Token(unsigned Offset, unsigned Length, tok::TokenKind Kind, +unsigned short Flags) + : Offset(Offset), Length(Length), Kind(Kind), Flags(Flags) {} + + unsigned getEnd() const { return Offset + Length; } + + bool is(tok::TokenKind K) const { return Kind == K; } + bool isNot(tok::TokenKind K) const { return Kind != K; } + bool isOneOf(tok::TokenKind K1, tok::TokenKind K2) const { +return is(K1) || is(K2); + } + template bool isOneOf(tok::TokenKind K1, Ts... Ks) const { +return is(K1) || isOneOf(Ks...); + } +}; + /// Represents the kind of preprocessor directive or a module declaration that /// is tracked by the scanner in its token output.
[clang] b58a420 - [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens
Author: Argyrios Kyrtzidis Date: 2022-05-26T12:49:51-07:00 New Revision: b58a420ff4f92b085fd718600fda162059171a58 URL: https://github.com/llvm/llvm-project/commit/b58a420ff4f92b085fd718600fda162059171a58 DIFF: https://github.com/llvm/llvm-project/commit/b58a420ff4f92b085fd718600fda162059171a58.diff LOG: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens This is first of a series of patches for making the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`. This patch only includes NFC renaming changes to make reviewing of the functionality changing parts easier. Differential Revision: https://reviews.llvm.org/D125484 Added: clang/include/clang/Lex/DependencyDirectivesScanner.h clang/lib/Lex/DependencyDirectivesScanner.cpp clang/test/ClangScanDeps/macro-expansions.cpp clang/unittests/Lex/DependencyDirectivesScannerTest.cpp Modified: clang/include/clang/Basic/DiagnosticLexKinds.td clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h clang/lib/Frontend/FrontendActions.cpp clang/lib/Lex/CMakeLists.txt clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp clang/test/ClangScanDeps/has_include_if_elif.cpp clang/test/ClangScanDeps/modulemap-via-vfs.m clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m clang/test/ClangScanDeps/modules-full-by-mod-name.cpp clang/test/ClangScanDeps/modules-full.cpp clang/test/ClangScanDeps/modules-inferred-explicit-build.m clang/test/ClangScanDeps/modules-inferred.m clang/test/ClangScanDeps/modules.cpp clang/test/ClangScanDeps/preprocess_minimized_pragmas.cpp clang/test/ClangScanDeps/regular_cdb.cpp clang/tools/clang-scan-deps/ClangScanDeps.cpp clang/unittests/Lex/CMakeLists.txt clang/unittests/Tooling/DependencyScannerTest.cpp Removed: clang/include/clang/Lex/DependencyDirectivesSourceMinimizer.h clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp clang/unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 4cfd314c01270..398fb15352fe6 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -899,11 +899,11 @@ def err_pp_eof_in_assume_nonnull : Error< } -let CategoryName = "Dependency Directive Source Minimization Issue" in { +let CategoryName = "Dependency Directive Source Scanner Issue" in { -def err_dep_source_minimizer_missing_sema_after_at_import : Error< +def err_dep_source_scanner_missing_semi_after_at_import : Error< "could not find ';' after @import">; -def err_dep_source_minimizer_unexpected_tokens_at_import : Error< +def err_dep_source_scanner_unexpected_tokens_at_import : Error< "unexpected extra tokens at end of @import declaration">; } diff --git a/clang/include/clang/Lex/DependencyDirectivesSourceMinimizer.h b/clang/include/clang/Lex/DependencyDirectivesScanner.h similarity index 73% rename from clang/include/clang/Lex/DependencyDirectivesSourceMinimizer.h rename to clang/include/clang/Lex/DependencyDirectivesScanner.h index 56025c8a3ed5d..b65891c6b8aba 100644 --- a/clang/include/clang/Lex/DependencyDirectivesSourceMinimizer.h +++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h @@ -1,4 +1,4 @@ -//===- clang/Lex/DependencyDirectivesSourceMinimizer.h - --*- C++ -*-// +//===- clang/Lex/DependencyDirectivesScanner.h -*- C++ -*-// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -7,15 +7,15 @@ //===--===// /// /// \file -/// This is the interface for minimizing header and source files to the +/// This is the interface for scanning header and source files to get the /// minimum necessary preprocessor directives for evaluating includes. It /// reduces the source down to #define, #include, #import, @import, and any /// conditional preprocessor logic that contains one of those. /// //===--===// -#ifndef LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSOURCEMINIMIZER_H -#define LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSOURCEMINIMIZER_H +#ifndef LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H +#define LLVM_CLANG_LEX_DEPENDENCYDIRECTIVESSCANNER_H #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/ArrayRef.h" @@ -26,11 +26,11 @@ namespace clang { class DiagnosticsEngine;
[clang] 42823be - [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default
Author: Argyrios Kyrtzidis Date: 2022-04-28T15:23:03-07:00 New Revision: 42823beb1d710f8e7273e3025e4ba793253fc0a4 URL: https://github.com/llvm/llvm-project/commit/42823beb1d710f8e7273e3025e4ba793253fc0a4 DIFF: https://github.com/llvm/llvm-project/commit/42823beb1d710f8e7273e3025e4ba793253fc0a4.diff LOG: [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default This is to improve maintenance a bit and remove need to maintain the additional option and related code-paths. Differential Revision: https://reviews.llvm.org/D124558 Added: Modified: clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp clang/test/ClangScanDeps/regular_cdb.cpp clang/tools/clang-scan-deps/ClangScanDeps.cpp clang/unittests/Tooling/DependencyScannerTest.cpp Removed: diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h index 7c830d3f27333..021a158530537 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningFilesystem.h @@ -293,7 +293,7 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { DependencyScanningWorkerFilesystem( DependencyScanningFilesystemSharedCache , IntrusiveRefCntPtr FS, - ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings) + ExcludedPreprocessorDirectiveSkipMapping ) : ProxyFileSystem(std::move(FS)), SharedCache(SharedCache), PPSkipMappings(PPSkipMappings) {} @@ -398,10 +398,10 @@ class DependencyScanningWorkerFilesystem : public llvm::vfs::ProxyFileSystem { /// The local cache is used by the worker thread to cache file system queries /// locally instead of querying the global cache every time. DependencyScanningFilesystemLocalCache LocalCache; - /// The optional mapping structure which records information about the + /// The mapping structure which records information about the /// excluded conditional directive skip mappings that are used by the /// currently active preprocessor. - ExcludedPreprocessorDirectiveSkipMapping *PPSkipMappings; + ExcludedPreprocessorDirectiveSkipMapping /// The set of files that should not be minimized. llvm::DenseSet NotToBeMinimized; }; diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h index 5c6dce611a952..79abe10887298 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h @@ -48,7 +48,6 @@ class DependencyScanningService { public: DependencyScanningService(ScanningMode Mode, ScanningOutputFormat Format, bool ReuseFileManager = true, -bool SkipExcludedPPRanges = true, bool OptimizeArgs = false); ScanningMode getMode() const { return Mode; } @@ -57,8 +56,6 @@ class DependencyScanningService { bool canReuseFileManager() const { return ReuseFileManager; } - bool canSkipExcludedPPRanges() const { return SkipExcludedPPRanges; } - bool canOptimizeArgs() const { return OptimizeArgs; } DependencyScanningFilesystemSharedCache () { @@ -69,10 +66,6 @@ class DependencyScanningService { const ScanningMode Mode; const ScanningOutputFormat Format; const bool ReuseFileManager; - /// Set to true to use the preprocessor optimization that skips excluded PP - /// ranges by bumping the buffer pointer in the lexer instead of lexing the - /// tokens in the range until reaching the corresponding directive. - const bool SkipExcludedPPRanges; /// Whether to optimize the modules' command-line arguments. const bool OptimizeArgs; /// The global file system cache. diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h index b7631c09f2759..84c74ac66359d 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h @@ -69,7 +69,7 @@ class DependencyScanningWorker { private: std::shared_ptr PCHContainerOps; - std::unique_ptr PPSkipMappings; +
[clang] f2b2490 - [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr`
Author: Argyrios Kyrtzidis Date: 2022-03-08T01:00:07-08:00 New Revision: f2b24905bfede6bd047a38f5cbae352e6b845428 URL: https://github.com/llvm/llvm-project/commit/f2b24905bfede6bd047a38f5cbae352e6b845428 DIFF: https://github.com/llvm/llvm-project/commit/f2b24905bfede6bd047a38f5cbae352e6b845428.diff LOG: [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr` Otherwise we will fail to generate the definition of a defaulted destructor, if the only reference was in a templated temporary. rdar://89366678 Differential Revision: https://reviews.llvm.org/D120426 Added: clang/test/SemaTemplate/defaulted-destructor-in-temporary.cpp Modified: clang/lib/Sema/TreeTransform.h Removed: diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 0716689d4b626..6676bffb8a47a 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -12748,6 +12748,9 @@ ExprResult TreeTransform::TransformCXXInheritedCtorInitExpr( template ExprResult TreeTransform::TransformCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) { + if (auto *Dtor = E->getTemporary()->getDestructor()) +SemaRef.MarkFunctionReferenced(E->getBeginLoc(), + const_cast(Dtor)); return getDerived().TransformExpr(E->getSubExpr()); } diff --git a/clang/test/SemaTemplate/defaulted-destructor-in-temporary.cpp b/clang/test/SemaTemplate/defaulted-destructor-in-temporary.cpp new file mode 100644 index 0..cbd4950c6af8e --- /dev/null +++ b/clang/test/SemaTemplate/defaulted-destructor-in-temporary.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -std=c++11 -triple=x86_64-apple-darwin %s -emit-llvm -o - | FileCheck %s + +// CHECK: define linkonce_odr {{.*}} @_ZN3StrD1Ev + +class A { +public: + ~A(); +}; +class Str { + A d; + +public: + ~Str() = default; +}; +class E { + Str s; + template + void h() { +s = {}; + } + void f(); +}; +void E::f() { + h(); +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a3a8a1a - [Index] Ignore nullptr decls for indexing
Author: Alex Hoppen Date: 2021-05-06T13:12:26-07:00 New Revision: a3a8a1a15b524d91b5308db68e9d293b34cd88dd URL: https://github.com/llvm/llvm-project/commit/a3a8a1a15b524d91b5308db68e9d293b34cd88dd DIFF: https://github.com/llvm/llvm-project/commit/a3a8a1a15b524d91b5308db68e9d293b34cd88dd.diff LOG: [Index] Ignore nullptr decls for indexing We can end up with a call to `indexTopLevelDecl(D)` with `D == nullptr` in non-assert builds e.g. when indexing a module in `indexModule` and - `ASTReader::GetDecl` returns `nullptr` if `Index >= DeclsLoaded.size()`, thus returning `nullptr` => `ModuleDeclIterator::operator*` returns `nullptr` => we call `IndexCtx.indexTopLevelDecl` with `nullptr` Be resilient and just ignore the `nullptr` decls during indexing. Reviewed By: akyrtzi Differential Revision: https://reviews.llvm.org/D102001 Added: Modified: clang/lib/Index/IndexDecl.cpp Removed: diff --git a/clang/lib/Index/IndexDecl.cpp b/clang/lib/Index/IndexDecl.cpp index 2ba323e63575..00adb3644ff2 100644 --- a/clang/lib/Index/IndexDecl.cpp +++ b/clang/lib/Index/IndexDecl.cpp @@ -759,7 +759,7 @@ bool IndexingContext::indexDeclContext(const DeclContext *DC) { } bool IndexingContext::indexTopLevelDecl(const Decl *D) { - if (D->getLocation().isInvalid()) + if (!D || D->getLocation().isInvalid()) return true; if (isa(D)) ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1206b95 - [ASTReader] Only mark module out of date if not already compiled
Author: Ben Barham Date: 2021-04-16T17:57:03-07:00 New Revision: 1206b95e0703dc0a9b619a095d5564ac51c39d19 URL: https://github.com/llvm/llvm-project/commit/1206b95e0703dc0a9b619a095d5564ac51c39d19 DIFF: https://github.com/llvm/llvm-project/commit/1206b95e0703dc0a9b619a095d5564ac51c39d19.diff LOG: [ASTReader] Only mark module out of date if not already compiled If a module contains errors (ie. it was built with -fallow-pcm-with-compiler-errors and had errors) and was from the module cache, it is marked as out of date - see a2c1054c303f20be006e9ef20739dbb88bd9ae02. When a module is imported multiple times in the one compile, this caused it to be recompiled each time - removing the existing buffer from the module cache and replacing it. This results in various errors further down the line. Instead, only mark the module as out of date if it isn't already finalized in the module cache. Reviewed By: akyrtzi Differential Revision: https://reviews.llvm.org/D100619 Added: clang/test/Modules/Inputs/error/use_error_a.h clang/test/Modules/Inputs/error/use_error_b.h Modified: clang/lib/Serialization/ASTReader.cpp clang/test/Modules/Inputs/error/error.h clang/test/Modules/Inputs/error/module.modulemap clang/test/Modules/load-module-with-errors.m Removed: diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 72bb125397dbd..88fb35aae1b8a 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2760,9 +2760,10 @@ ASTReader::ReadControlBlock(ModuleFile , bool hasErrors = Record[6]; if (hasErrors && !DisableValidation) { -// If requested by the caller, mark modules on error as out-of-date. -if (F.Kind == MK_ImplicitModule && -(ClientLoadCapabilities & ARR_TreatModuleWithErrorsAsOutOfDate)) +// If requested by the caller and the module hasn't already been read +// or compiled, mark modules on error as out-of-date. +if ((ClientLoadCapabilities & ARR_TreatModuleWithErrorsAsOutOfDate) && +!ModuleMgr.getModuleCache().isPCMFinal(F.FileName)) return OutOfDate; if (!AllowASTWithCompilerErrors) { diff --git a/clang/test/Modules/Inputs/error/error.h b/clang/test/Modules/Inputs/error/error.h index 1b27b21dfd635..fcdb408a4534f 100644 --- a/clang/test/Modules/Inputs/error/error.h +++ b/clang/test/Modules/Inputs/error/error.h @@ -1,3 +1,5 @@ +#pragma mark mark + @import undefined; @interface Error diff --git a/clang/test/Modules/Inputs/error/module.modulemap b/clang/test/Modules/Inputs/error/module.modulemap index e18239af113b2..512e7ed6529e0 100644 --- a/clang/test/Modules/Inputs/error/module.modulemap +++ b/clang/test/Modules/Inputs/error/module.modulemap @@ -1,3 +1,13 @@ module error { header "error.h" } + +module use_error_a { + header "use_error_a.h" + export error +} + +module use_error_b { + header "use_error_b.h" + export error +} diff --git a/clang/test/Modules/Inputs/error/use_error_a.h b/clang/test/Modules/Inputs/error/use_error_a.h new file mode 100644 index 0..1949c8346041d --- /dev/null +++ b/clang/test/Modules/Inputs/error/use_error_a.h @@ -0,0 +1,3 @@ +@import error; + +void funca(Error *a); diff --git a/clang/test/Modules/Inputs/error/use_error_b.h b/clang/test/Modules/Inputs/error/use_error_b.h new file mode 100644 index 0..025acb7ccf31e --- /dev/null +++ b/clang/test/Modules/Inputs/error/use_error_b.h @@ -0,0 +1,3 @@ +@import error; + +void funcb(Error *b); diff --git a/clang/test/Modules/load-module-with-errors.m b/clang/test/Modules/load-module-with-errors.m index 3a951d2cdaa62..6991d0feb0103 100644 --- a/clang/test/Modules/load-module-with-errors.m +++ b/clang/test/Modules/load-module-with-errors.m @@ -2,10 +2,13 @@ // matter in this test. // pcherror-error@* {{PCH file contains compiler errors}} -@import error; // notallowerror-error {{could not build module 'error'}} +@import use_error_a; // notallowerror-error {{could not build module 'use_error_a'}} +@import use_error_b; // expected-no-diagnostics void test(Error *x) { + funca(x); + funcb(x); [x method]; } @@ -16,7 +19,16 @@ void test(Error *x) { // RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \ // RUN: -fmodule-name=error -o %t/prebuilt/error.pcm \ // RUN: -x objective-c -emit-module %S/Inputs/error/module.modulemap +// RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \ +// RUN: -fmodule-file=error=%t/prebuilt/error.pcm \ +// RUN: -fmodule-name=use_error_a -o %t/prebuilt/use_error_a.pcm \ +// RUN: -x objective-c -emit-module %S/Inputs/error/module.modulemap +// RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \ +// RUN: -fmodule-file=error=%t/prebuilt/error.pcm \ +// RUN: -fmodule-name=use_error_b -o %t/prebuilt/use_error_b.pcm \ +//
[clang] a8cb39b - Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules
Author: Argyrios Kyrtzidis Date: 2021-02-08T16:10:39-08:00 New Revision: a8cb39bab04c317c9886ec3a332f3b70ce27ae4f URL: https://github.com/llvm/llvm-project/commit/a8cb39bab04c317c9886ec3a332f3b70ce27ae4f DIFF: https://github.com/llvm/llvm-project/commit/a8cb39bab04c317c9886ec3a332f3b70ce27ae4f.diff LOG: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules A module with errors would be marked as out-of-date, then the `compilerModule` action would produce it, but due to the error it would be treated as failure and the resulting PCM would not get used. rdar://74087062 Differential Revision: https://reviews.llvm.org/D96246 Added: Modified: clang/include/clang/Serialization/ASTReader.h clang/lib/Frontend/CompilerInstance.cpp clang/lib/Serialization/ASTReader.cpp clang/test/Modules/load-module-with-errors.m Removed: diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index d0d2a68114c7..0df687c05366 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1543,7 +1543,11 @@ class ASTReader /// The client can handle an AST file that cannot load because it's /// compiled configuration doesn't match that of the context it was /// loaded into. -ARR_ConfigurationMismatch = 0x8 +ARR_ConfigurationMismatch = 0x8, + +/// If a module file is marked with errors treat it as out-of-date so the +/// caller can rebuild it. +ARR_TreatModuleWithErrorsAsOutOfDate = 0x10 }; /// Load the AST file designated by the given file name. diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 956877d34680..7c2b2bf57917 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1144,7 +1144,10 @@ compileModuleImpl(CompilerInstance , SourceLocation ImportLoc, // module generation thread crashed. Instance.clearOutputFiles(/*EraseFiles=*/true); - return !Instance.getDiagnostics().hasErrorOccurred(); + // If \p AllowPCMWithCompilerErrors is set return 'success' even if errors + // occurred. + return !Instance.getDiagnostics().hasErrorOccurred() || + Instance.getFrontendOpts().AllowPCMWithCompilerErrors; } static const FileEntry *getPublicModuleMap(const FileEntry *File, @@ -1697,7 +1700,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( // Try to load the module file. If we are not trying to load from the // module cache, we don't know how to rebuild modules. unsigned ARRFlags = Source == MS_ModuleCache - ? ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing + ? ASTReader::ARR_OutOfDate | ASTReader::ARR_Missing | +ASTReader::ARR_TreatModuleWithErrorsAsOutOfDate : Source == MS_PrebuiltModulePath ? 0 : ASTReader::ARR_ConfigurationMismatch; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 99579f7956ed..e3d3938ac9d6 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2751,8 +2751,9 @@ ASTReader::ReadControlBlock(ModuleFile , bool hasErrors = Record[6]; if (hasErrors && !DisableValidation) { -// Always rebuild modules from the cache on an error -if (F.Kind == MK_ImplicitModule) +// If requested by the caller, mark modules on error as out-of-date. +if (F.Kind == MK_ImplicitModule && +(ClientLoadCapabilities & ARR_TreatModuleWithErrorsAsOutOfDate)) return OutOfDate; if (!AllowASTWithCompilerErrors) { diff --git a/clang/test/Modules/load-module-with-errors.m b/clang/test/Modules/load-module-with-errors.m index 3dceb545ad9c..3a951d2cdaa6 100644 --- a/clang/test/Modules/load-module-with-errors.m +++ b/clang/test/Modules/load-module-with-errors.m @@ -1,3 +1,14 @@ +// Note: the run lines follow their respective tests, since line/column +// matter in this test. + +// pcherror-error@* {{PCH file contains compiler errors}} +@import error; // notallowerror-error {{could not build module 'error'}} +// expected-no-diagnostics + +void test(Error *x) { + [x method]; +} + // RUN: rm -rf %t // RUN: mkdir %t // RUN: mkdir %t/prebuilt @@ -48,21 +59,27 @@ // the verify would fail as it would be the PCH error instead) // RUN: %clang_cc1 -fsyntax-only -fmodules \ // RUN: -fmodules-cache-path=%t -fimplicit-module-maps -I %S/Inputs/error \ -// RUN: -x objective-c -verify %s +// RUN: -x objective-c -verify=notallowerror %s // allow-pcm-with-compiler-errors should also allow errors in PCH // RUN: %clang_cc1
[clang] a2c1054 - [ASTReader] Always rebuild a cached module that has errors
Author: Ben Barham Date: 2021-02-03T22:06:46-08:00 New Revision: a2c1054c303f20be006e9ef20739dbb88bd9ae02 URL: https://github.com/llvm/llvm-project/commit/a2c1054c303f20be006e9ef20739dbb88bd9ae02 DIFF: https://github.com/llvm/llvm-project/commit/a2c1054c303f20be006e9ef20739dbb88bd9ae02.diff LOG: [ASTReader] Always rebuild a cached module that has errors A module in the cache with an error should just be a cache miss. If allowing errors (with -fallow-pcm-with-compiler-errors), a rebuild is needed so that the appropriate diagnostics are output and in case search paths have changed. If not allowing errors, the module was built *allowing* errors and thus should be rebuilt regardless. Reviewed By: akyrtzi Differential Revision: https://reviews.llvm.org/D95989 Added: clang/test/Modules/Inputs/error/error.h clang/test/Modules/Inputs/error/module.modulemap Modified: clang/lib/Serialization/ASTReader.cpp clang/test/Modules/Inputs/module.map clang/test/Modules/load-module-with-errors.m Removed: clang/test/Modules/Inputs/error.h diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 85add2e2740b..99579f7956ed 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -2750,9 +2750,15 @@ ASTReader::ReadControlBlock(ModuleFile , } bool hasErrors = Record[6]; - if (hasErrors && !DisableValidation && !AllowASTWithCompilerErrors) { -Diag(diag::err_pch_with_compiler_errors); -return HadErrors; + if (hasErrors && !DisableValidation) { +// Always rebuild modules from the cache on an error +if (F.Kind == MK_ImplicitModule) + return OutOfDate; + +if (!AllowASTWithCompilerErrors) { + Diag(diag::err_pch_with_compiler_errors); + return HadErrors; +} } if (hasErrors) { Diags.ErrorOccurred = true; diff --git a/clang/test/Modules/Inputs/error.h b/clang/test/Modules/Inputs/error.h deleted file mode 100644 index 7201003d28c3.. --- a/clang/test/Modules/Inputs/error.h +++ /dev/null @@ -1,8 +0,0 @@ -@import undefined - -@interface Error -- (int)method; -undefined -@end - -undefined diff --git a/clang/test/Modules/Inputs/error/error.h b/clang/test/Modules/Inputs/error/error.h new file mode 100644 index ..1b27b21dfd63 --- /dev/null +++ b/clang/test/Modules/Inputs/error/error.h @@ -0,0 +1,9 @@ +@import undefined; + +@interface Error +- (int)method; +- (undefined)method2; +undefined; +@end + +undefined diff --git a/clang/test/Modules/Inputs/error/module.modulemap b/clang/test/Modules/Inputs/error/module.modulemap new file mode 100644 index ..e18239af113b --- /dev/null +++ b/clang/test/Modules/Inputs/error/module.modulemap @@ -0,0 +1,3 @@ +module error { + header "error.h" +} diff --git a/clang/test/Modules/Inputs/module.map b/clang/test/Modules/Inputs/module.map index 2aa0733537bc..e7cb4b27bc08 100644 --- a/clang/test/Modules/Inputs/module.map +++ b/clang/test/Modules/Inputs/module.map @@ -483,4 +483,3 @@ module template_nontrivial1 { header "template-nontrivial1.h" export * } -module error { header "error.h" } diff --git a/clang/test/Modules/load-module-with-errors.m b/clang/test/Modules/load-module-with-errors.m index 64575482d091..3dceb545ad9c 100644 --- a/clang/test/Modules/load-module-with-errors.m +++ b/clang/test/Modules/load-module-with-errors.m @@ -1,25 +1,68 @@ // RUN: rm -rf %t // RUN: mkdir %t +// RUN: mkdir %t/prebuilt -// Write out a module with errors make sure it can be read // RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \ -// RUN: -fmodules-cache-path=%t -x objective-c -emit-module \ -// RUN: -fmodule-name=error %S/Inputs/module.map -// RUN: %clang_cc1 -fmodules -fallow-pcm-with-compiler-errors \ -// RUN: -fmodules-cache-path=%t -x objective-c -I %S/Inputs \ -// RUN: -fimplicit-module-maps -ast-print %s | FileCheck %s +// RUN: -fmodule-name=error -o %t/prebuilt/error.pcm \ +// RUN: -x objective-c -emit-module %S/Inputs/error/module.modulemap + +// RUN: %clang_cc1 -fsyntax-only -fmodules -fallow-pcm-with-compiler-errors \ +// RUN: -fprebuilt-module-path=%t/prebuilt -fmodules-cache-path=%t \ +// RUN: -ast-print %s | FileCheck %s +// RUN: %clang_cc1 -fsyntax-only -fmodules \ +// RUN: -fprebuilt-module-path=%t/prebuilt -fmodules-cache-path=%t \ +// RUN: -verify=pcherror %s + +// RUN: %clang_cc1 -fsyntax-only -fmodules -fallow-pcm-with-compiler-errors \ +// RUN: -fmodule-file=error=%t/prebuilt/error.pcm -fmodules-cache-path=%t \ +// RUN: -ast-print %s | FileCheck %s +// RUN: %clang_cc1 -fsyntax-only -fmodules \ +// RUN: -fmodule-file=error=%t/prebuilt/error.pcm -fmodules-cache-path=%t \ +// RUN: -verify=pcherror %s + +// RUN: %clang_cc1 -fsyntax-only -fmodules -fallow-pcm-with-compiler-errors \ +//
[clang] b0e8990 - [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file
Author: Argyrios Kyrtzidis Date: 2021-01-21T20:45:54-08:00 New Revision: b0e89906f5b7e505a1ea315ab4ff612b1607fda8 URL: https://github.com/llvm/llvm-project/commit/b0e89906f5b7e505a1ea315ab4ff612b1607fda8 DIFF: https://github.com/llvm/llvm-project/commit/b0e89906f5b7e505a1ea315ab4ff612b1607fda8.diff LOG: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file This addresses an issue with how the PCH preable works, specifically: 1. When using a PCH/preamble the module hash changes and a different cache directory is used 2. When the preamble is used, PCH & PCM validation is disabled. Due to combination of #1 and #2, reparsing with preamble enabled can end up loading a stale module file before a header change and using it without updating it because validation is disabled and it doesn’t check that the header has changed and the module file is out-of-date. rdar://72611253 Differential Revision: https://reviews.llvm.org/D95159 Added: clang/test/Index/Inputs/preamble-reparse-changed-module/head.h clang/test/Index/Inputs/preamble-reparse-changed-module/module.modulemap clang/test/Index/Inputs/preamble-reparse-changed-module/new-head.h clang/test/Index/preamble-reparse-changed-module.m Modified: clang/include/clang/Driver/Options.td clang/include/clang/Frontend/CompilerInstance.h clang/include/clang/Lex/PreprocessorOptions.h clang/include/clang/Serialization/ASTReader.h clang/lib/Frontend/ASTUnit.cpp clang/lib/Frontend/ChainedIncludesSource.cpp clang/lib/Frontend/CompilerInstance.cpp clang/lib/Frontend/FrontendAction.cpp clang/lib/Frontend/FrontendActions.cpp clang/lib/Frontend/PrecompiledPreamble.cpp clang/lib/Serialization/ASTReader.cpp clang/tools/c-index-test/c-index-test.c clang/tools/c-index-test/core_main.cpp Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 500022c2c99b..a2800381be0e 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -5121,7 +5121,8 @@ def fhalf_no_semantic_interposition : Flag<["-"], "fhalf-no-semantic-interpositi MarshallingInfoFlag>; def fno_validate_pch : Flag<["-"], "fno-validate-pch">, HelpText<"Disable validation of precompiled headers">, - MarshallingInfoFlag>; + MarshallingInfoFlag, "DisableValidationForModuleKind::None">, + Normalizer<"makeFlagToValueNormalizer(DisableValidationForModuleKind::All)">; def fallow_pcm_with_errors : Flag<["-"], "fallow-pcm-with-compiler-errors">, HelpText<"Accept a PCM file that was created with compiler errors">, MarshallingInfoFlag>; diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 4fc002c6f383..15f6ed2df885 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -50,6 +50,7 @@ class Preprocessor; class Sema; class SourceManager; class TargetInfo; +enum class DisableValidationForModuleKind; /// CompilerInstance - Helper class for managing a single instance of the Clang /// compiler. @@ -659,16 +660,17 @@ class CompilerInstance : public ModuleLoader { /// Create an external AST source to read a PCH file and attach it to the AST /// context. - void createPCHExternalASTSource(StringRef Path, bool DisablePCHValidation, - bool AllowPCHWithCompilerErrors, - void *DeserializationListener, - bool OwnDeserializationListener); + void createPCHExternalASTSource( + StringRef Path, DisableValidationForModuleKind DisableValidation, + bool AllowPCHWithCompilerErrors, void *DeserializationListener, + bool OwnDeserializationListener); /// Create an external AST source to read a PCH file. /// /// \return - The new object on success, or null on failure. static IntrusiveRefCntPtr createPCHExternalASTSource( - StringRef Path, StringRef Sysroot, bool DisablePCHValidation, + StringRef Path, StringRef Sysroot, + DisableValidationForModuleKind DisableValidation, bool AllowPCHWithCompilerErrors, Preprocessor , InMemoryModuleCache , ASTContext , const PCHContainerReader , diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h index c551f87e0d7b..7f024989bf9b 100644 --- a/clang/include/clang/Lex/PreprocessorOptions.h +++ b/clang/include/clang/Lex/PreprocessorOptions.h @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_LEX_PREPROCESSOROPTIONS_H_ #define LLVM_CLANG_LEX_PREPROCESSOROPTIONS_H_ +#include "clang/Basic/BitmaskEnum.h" #include "clang/Basic/LLVM.h" #include "clang/Lex/PreprocessorExcludedConditionalDirectiveSkipMapping.h" #include "llvm/ADT/StringRef.h" @@ -40,6 +41,24
[clang] 5834996 - [Frontend] Add flag to allow PCM generation despite compiler errors
Author: Ben Barham Date: 2020-11-17T17:27:50-08:00 New Revision: 5834996fefc937d6211dc8c8a5b200068753391a URL: https://github.com/llvm/llvm-project/commit/5834996fefc937d6211dc8c8a5b200068753391a DIFF: https://github.com/llvm/llvm-project/commit/5834996fefc937d6211dc8c8a5b200068753391a.diff LOG: [Frontend] Add flag to allow PCM generation despite compiler errors As with precompiled headers, it's useful for indexers to be able to continue through compiler errors in dependent modules. Resolves rdar://69816264 Reviewed By: akyrtzi Differential Revision: https://reviews.llvm.org/D91580 Added: clang/test/Modules/Inputs/error.h clang/test/Modules/load-module-with-errors.m Modified: clang/include/clang/Driver/Options.td clang/include/clang/Frontend/ASTUnit.h clang/include/clang/Frontend/FrontendActions.h clang/include/clang/Frontend/FrontendOptions.h clang/lib/Frontend/ASTUnit.cpp clang/lib/Frontend/CompilerInstance.cpp clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Frontend/FrontendActions.cpp clang/test/Modules/Inputs/module.map clang/tools/c-index-test/core_main.cpp clang/tools/libclang/CIndex.cpp Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 0168d7000737..feae81317047 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4356,6 +4356,8 @@ def fno_validate_pch : Flag<["-"], "fno-validate-pch">, HelpText<"Disable validation of precompiled headers">; def fallow_pch_with_errors : Flag<["-"], "fallow-pch-with-compiler-errors">, HelpText<"Accept a PCH file that was created with compiler errors">; +def fallow_pcm_with_errors : Flag<["-"], "fallow-pcm-with-compiler-errors">, + HelpText<"Accept a PCM file that was created with compiler errors">; def dump_deserialized_pch_decls : Flag<["-"], "dump-deserialized-decls">, HelpText<"Dump declarations that are deserialized from PCH, for testing">; def error_on_deserialized_pch_decl : Separate<["-"], "error-on-deserialized-decl">, diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h index 5595b8c7bcae..5bee57042ca6 100644 --- a/clang/include/clang/Frontend/ASTUnit.h +++ b/clang/include/clang/Frontend/ASTUnit.h @@ -694,7 +694,7 @@ class ASTUnit { const FileSystemOptions , bool UseDebugInfo = false, bool OnlyLocalDecls = false, ArrayRef RemappedFiles = None, CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None, - bool AllowPCHWithCompilerErrors = false, + bool AllowASTWithCompilerErrors = false, bool UserFilesAreVolatile = false); private: diff --git a/clang/include/clang/Frontend/FrontendActions.h b/clang/include/clang/Frontend/FrontendActions.h index 9ca2bfda2138..25ca95980806 100644 --- a/clang/include/clang/Frontend/FrontendActions.h +++ b/clang/include/clang/Frontend/FrontendActions.h @@ -117,6 +117,8 @@ class GenerateModuleAction : public ASTFrontendAction { } bool hasASTFileSupport() const override { return false; } + + bool shouldEraseOutputFiles() override; }; class GenerateInterfaceStubsAction : public ASTFrontendAction { diff --git a/clang/include/clang/Frontend/FrontendOptions.h b/clang/include/clang/Frontend/FrontendOptions.h index 51dc3a0dc7f1..b06ad5203e75 100644 --- a/clang/include/clang/Frontend/FrontendOptions.h +++ b/clang/include/clang/Frontend/FrontendOptions.h @@ -303,6 +303,9 @@ class FrontendOptions { /// When using -emit-module, treat the modulemap as a system module. unsigned IsSystemModule : 1; + /// Output (and read) PCM files regardless of compiler errors. + unsigned AllowPCMWithCompilerErrors : 1; + CodeCompleteOptions CodeCompleteOpts; /// Specifies the output format of the AST. @@ -457,7 +460,8 @@ class FrontendOptions { UseGlobalModuleIndex(true), GenerateGlobalModuleIndex(true), ASTDumpDecls(false), ASTDumpLookups(false), BuildingImplicitModule(false), ModulesEmbedAllFiles(false), -IncludeTimestamps(true), UseTemporary(true), TimeTraceGranularity(500) {} +IncludeTimestamps(true), UseTemporary(true), +AllowPCMWithCompilerErrors(false), TimeTraceGranularity(500) {} /// getInputKindForExtension - Return the appropriate input kind for a file /// extension. For example, "c" would return Language::C. diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index a112117d1c06..c2aba4102361 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -759,7 +759,7 @@ std::unique_ptr ASTUnit::LoadFromASTFile( WhatToLoad ToLoad, IntrusiveRefCntPtr Diags, const FileSystemOptions , bool UseDebugInfo, bool OnlyLocalDecls, ArrayRef RemappedFiles, -CaptureDiagsKind CaptureDiagnostics, bool AllowPCHWithCompilerErrors, +
[clang] fbb499e - [AST] Fix crashes caused by redeclarations in hidden prototypes
Author: Ben Barham Date: 2020-10-08T19:48:36-07:00 New Revision: fbb499ef255b77c5a3300543de88956b13e706b7 URL: https://github.com/llvm/llvm-project/commit/fbb499ef255b77c5a3300543de88956b13e706b7 DIFF: https://github.com/llvm/llvm-project/commit/fbb499ef255b77c5a3300543de88956b13e706b7.diff LOG: [AST] Fix crashes caused by redeclarations in hidden prototypes ObjCContainerDecl.getMethod returns a nullptr by default when the container is a hidden prototype. Callsites where the method is being looked up on the redeclaration's own container should skip this check since they (rightly) expect a valid method to be found. Resolves rdar://69444243 Reviewed By: akyrtzi Differential Revision: https://reviews.llvm.org/D89024 Added: clang/test/Index/Inputs/hidden-redecls-sub.h clang/test/Index/Inputs/hidden-redecls.h clang/test/Index/hidden-redecls.m Modified: clang/lib/AST/DeclObjC.cpp clang/test/Index/Inputs/module.map Removed: diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp index b6f8227b157a..961230fb54ce 100644 --- a/clang/lib/AST/DeclObjC.cpp +++ b/clang/lib/AST/DeclObjC.cpp @@ -950,7 +950,8 @@ ObjCMethodDecl *ObjCMethodDecl::getNextRedeclarationImpl() { if (!Redecl && isRedeclaration()) { // This is the last redeclaration, go back to the first method. return cast(CtxD)->getMethod(getSelector(), -isInstanceMethod()); +isInstanceMethod(), +/*AllowHidden=*/true); } return Redecl ? Redecl : this; @@ -983,7 +984,8 @@ ObjCMethodDecl *ObjCMethodDecl::getCanonicalDecl() { if (isRedeclaration()) { // It is possible that we have not done deserializing the ObjCMethod yet. ObjCMethodDecl *MD = -cast(CtxD)->getMethod(Sel, isInstanceMethod()); +cast(CtxD)->getMethod(Sel, isInstanceMethod(), + /*AllowHidden=*/true); return MD ? MD : this; } @@ -1308,8 +1310,9 @@ void ObjCMethodDecl::getOverriddenMethods( const ObjCMethodDecl *Method = this; if (Method->isRedeclaration()) { -Method = cast(Method->getDeclContext())-> - getMethod(Method->getSelector(), Method->isInstanceMethod()); +Method = cast(Method->getDeclContext()) + ->getMethod(Method->getSelector(), Method->isInstanceMethod(), + /*AllowHidden=*/true); } if (Method->isOverriding()) { diff --git a/clang/test/Index/Inputs/hidden-redecls-sub.h b/clang/test/Index/Inputs/hidden-redecls-sub.h new file mode 100644 index ..f5a77977cfba --- /dev/null +++ b/clang/test/Index/Inputs/hidden-redecls-sub.h @@ -0,0 +1,7 @@ +@protocol P1 +- (void)p1_method; +- (void)p1_method; +@end + +@interface Foo (SubP1) +@end diff --git a/clang/test/Index/Inputs/hidden-redecls.h b/clang/test/Index/Inputs/hidden-redecls.h new file mode 100644 index ..c5558cf0ab18 --- /dev/null +++ b/clang/test/Index/Inputs/hidden-redecls.h @@ -0,0 +1,3 @@ +@interface Foo +- (void)parent_method; +@end diff --git a/clang/test/Index/Inputs/module.map b/clang/test/Index/Inputs/module.map index 10712accb1c2..cd5bcb467032 100644 --- a/clang/test/Index/Inputs/module.map +++ b/clang/test/Index/Inputs/module.map @@ -20,3 +20,11 @@ module PreambleWithImplicitImport { export * } } + +module hidden_redecls { + header "hidden-redecls.h" + + explicit module sub { +header "hidden-redecls-sub.h" + } +} diff --git a/clang/test/Index/hidden-redecls.m b/clang/test/Index/hidden-redecls.m new file mode 100644 index ..1735c0b5e184 --- /dev/null +++ b/clang/test/Index/hidden-redecls.m @@ -0,0 +1,12 @@ +@import hidden_redecls; + +@interface Foo (Top) +- (void)top_method; +@end + +// p1_method in protocol P1 is hidden since module_redecls.sub hasn't been +// imported yet. Check it is still indexed. + +// RUN: c-index-test -index-file-full %s -isystem %S/Inputs -fmodules -target x86_64-apple-macosx10.7 | FileCheck %s +// CHECK: [indexDeclaration]: kind: objc-instance-method | name: p1_method | {{.*}} | loc: {{.*}}hidden-redecls-sub.h:2:9 | {{.*}} | isRedecl: 0 +// CHECK: [indexDeclaration]: kind: objc-instance-method | name: p1_method | {{.*}} | loc: {{.*}}hidden-redecls-sub.h:3:9 | {{.*}} | isRedecl: 1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 2b17438 - [Index/USRGeneration] Make sure that ObjC properties in categories also get namescoped properly for USR generation
Author: Argyrios Kyrtzidis Date: 2020-03-07T15:07:37-08:00 New Revision: 2b17438a92ea1ea178d9e14219a8e6ba01d4f04d URL: https://github.com/llvm/llvm-project/commit/2b17438a92ea1ea178d9e14219a8e6ba01d4f04d DIFF: https://github.com/llvm/llvm-project/commit/2b17438a92ea1ea178d9e14219a8e6ba01d4f04d.diff LOG: [Index/USRGeneration] Make sure that ObjC properties in categories also get namescoped properly for USR generation If the property is in a category that has module names from external_declaration property, make sure they are included in the USR. rdar://59897320 Added: Modified: clang/lib/Index/USRGeneration.cpp clang/test/Index/Core/external-source-symbol-attr.m Removed: diff --git a/clang/lib/Index/USRGeneration.cpp b/clang/lib/Index/USRGeneration.cpp index 394daf94c4b2..f3eb653f10fa 100644 --- a/clang/lib/Index/USRGeneration.cpp +++ b/clang/lib/Index/USRGeneration.cpp @@ -382,6 +382,14 @@ void USRGenerator::VisitNamespaceAliasDecl(const NamespaceAliasDecl *D) { Out << "@NA@" << D->getName(); } +static const ObjCCategoryDecl *getCategoryContext(const NamedDecl *D) { + if (auto *CD = dyn_cast(D->getDeclContext())) +return CD; + if (auto *ICD = dyn_cast(D->getDeclContext())) +return ICD->getCategoryDecl(); + return nullptr; +}; + void USRGenerator::VisitObjCMethodDecl(const ObjCMethodDecl *D) { const DeclContext *container = D->getDeclContext(); if (const ObjCProtocolDecl *pd = dyn_cast(container)) { @@ -395,14 +403,6 @@ void USRGenerator::VisitObjCMethodDecl(const ObjCMethodDecl *D) { IgnoreResults = true; return; } -auto getCategoryContext = [](const ObjCMethodDecl *D) -> -const ObjCCategoryDecl * { - if (auto *CD = dyn_cast(D->getDeclContext())) -return CD; - if (auto *ICD = dyn_cast(D->getDeclContext())) -return ICD->getCategoryDecl(); - return nullptr; -}; auto *CD = getCategoryContext(D); VisitObjCContainerDecl(ID, CD); } @@ -475,7 +475,7 @@ void USRGenerator::VisitObjCPropertyDecl(const ObjCPropertyDecl *D) { // The USR for a property declared in a class extension or category is based // on the ObjCInterfaceDecl, not the ObjCCategoryDecl. if (const ObjCInterfaceDecl *ID = Context->getObjContainingInterface(D)) -Visit(ID); +VisitObjCContainerDecl(ID, getCategoryContext(D)); else Visit(cast(D->getDeclContext())); GenObjCProperty(D->getName(), D->isClassProperty()); diff --git a/clang/test/Index/Core/external-source-symbol-attr.m b/clang/test/Index/Core/external-source-symbol-attr.m index 41bb7a264ab3..d2cef35ffab2 100644 --- a/clang/test/Index/Core/external-source-symbol-attr.m +++ b/clang/test/Index/Core/external-source-symbol-attr.m @@ -21,6 +21,10 @@ @interface I2 // CHECK: [[@LINE-1]]:12 | class/Swift | I2 | c:@M@some_module@objc(cs)I2 | {{.*}} | Decl | rel: 0 -(void)method; // CHECK: [[@LINE-1]]:8 | instance-method/Swift | method | c:@M@some_module@objc(cs)I2(im)method | -[I2 method] | Decl,Dyn,RelChild | rel: 1 +@property int prop; +// CHECK: [[@LINE-1]]:15 | instance-method/acc-get/Swift | prop | c:@M@some_module@objc(cs)I2(im)prop | +// CHECK: [[@LINE-2]]:15 | instance-method/acc-set/Swift | setProp: | c:@M@some_module@objc(cs)I2(im)setProp: | +// CHECK: [[@LINE-3]]:15 | instance-property/Swift | prop | c:@M@some_module@objc(cs)I2(py)prop | @end void test1(I1 *o) { @@ -45,6 +49,10 @@ @interface I1(cat2) // CHECK: [[@LINE-1]]:15 | extension/Swift | cat2 | c:@CM@cat_module@some_module@objc(cy)I1@cat2 | -(void)cat_method2; // CHECK: [[@LINE-1]]:8 | instance-method/Swift | cat_method2 | c:@CM@cat_module@some_module@objc(cs)I1(im)cat_method2 +@property int cat_prop2; +// CHECK: [[@LINE-1]]:15 | instance-method/acc-get/Swift | cat_prop2 | c:@CM@cat_module@some_module@objc(cs)I1(im)cat_prop2 | +// CHECK: [[@LINE-2]]:15 | instance-method/acc-set/Swift | setCat_prop2: | c:@CM@cat_module@some_module@objc(cs)I1(im)setCat_prop2: | +// CHECK: [[@LINE-3]]:15 | instance-property/Swift | cat_prop2 | c:@CM@cat_module@some_module@objc(cs)I1(py)cat_prop2 | @end #define NS_ENUM(_name, _type) enum _name:_type _name; enum _name : _type ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r342484 - [index] Enhance indexing for module references
Author: akirtzidis Date: Tue Sep 18 08:02:56 2018 New Revision: 342484 URL: http://llvm.org/viewvc/llvm-project?rev=342484=rev Log: [index] Enhance indexing for module references * Create a USR for the occurrences of the 'module' symbol kind * Record module references for each identifier in an import declaration Added: cfe/trunk/test/Index/Core/Inputs/module/SubModA.h cfe/trunk/test/Index/Core/Inputs/module/SubSubModA.h Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h cfe/trunk/include/clang/Index/USRGeneration.h cfe/trunk/lib/Index/IndexingAction.cpp cfe/trunk/lib/Index/IndexingContext.cpp cfe/trunk/lib/Index/USRGeneration.cpp cfe/trunk/test/Index/Core/Inputs/module/module.modulemap cfe/trunk/test/Index/Core/index-with-module.m cfe/trunk/tools/c-index-test/core_main.cpp cfe/trunk/tools/libclang/CXIndexDataConsumer.cpp cfe/trunk/tools/libclang/CXIndexDataConsumer.h Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexDataConsumer.h?rev=342484=342483=342484=diff == --- cfe/trunk/include/clang/Index/IndexDataConsumer.h (original) +++ cfe/trunk/include/clang/Index/IndexDataConsumer.h Tue Sep 18 08:02:56 2018 @@ -50,7 +50,12 @@ public: SourceLocation Loc); /// \returns true to continue indexing, or false to abort. + /// + /// This will be called for each module reference in an import decl. + /// For "@import MyMod.SubMod", there will be a call for 'MyMod' with the + /// 'reference' role, and a call for 'SubMod' with the 'declaration' role. virtual bool handleModuleOccurence(const ImportDecl *ImportD, + const Module *Mod, SymbolRoleSet Roles, SourceLocation Loc); virtual void finish() {} Modified: cfe/trunk/include/clang/Index/USRGeneration.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/USRGeneration.h?rev=342484=342483=342484=diff == --- cfe/trunk/include/clang/Index/USRGeneration.h (original) +++ cfe/trunk/include/clang/Index/USRGeneration.h Tue Sep 18 08:02:56 2018 @@ -16,6 +16,7 @@ namespace clang { class Decl; class MacroDefinitionRecord; +class Module; class SourceLocation; class SourceManager; @@ -70,6 +71,22 @@ bool generateUSRForMacro(const MacroDefi bool generateUSRForMacro(StringRef MacroName, SourceLocation Loc, const SourceManager , SmallVectorImpl ); +/// Generate a USR for a module, including the USR prefix. +/// \returns true on error, false on success. +bool generateFullUSRForModule(const Module *Mod, raw_ostream ); + +/// Generate a USR for a top-level module name, including the USR prefix. +/// \returns true on error, false on success. +bool generateFullUSRForTopLevelModuleName(StringRef ModName, raw_ostream ); + +/// Generate a USR fragment for a module. +/// \returns true on error, false on success. +bool generateUSRFragmentForModule(const Module *Mod, raw_ostream ); + +/// Generate a USR fragment for a module name. +/// \returns true on error, false on success. +bool generateUSRFragmentForModuleName(StringRef ModName, raw_ostream ); + } // namespace index } // namespace clang Modified: cfe/trunk/lib/Index/IndexingAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingAction.cpp?rev=342484=342483=342484=diff == --- cfe/trunk/lib/Index/IndexingAction.cpp (original) +++ cfe/trunk/lib/Index/IndexingAction.cpp Tue Sep 18 08:02:56 2018 @@ -37,6 +37,7 @@ bool IndexDataConsumer::handleMacroOccur } bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD, + const Module *Mod, SymbolRoleSet Roles, SourceLocation Loc) { return true; Modified: cfe/trunk/lib/Index/IndexingContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=342484=342483=342484=diff == --- cfe/trunk/lib/Index/IndexingContext.cpp (original) +++ cfe/trunk/lib/Index/IndexingContext.cpp Tue Sep 18 08:02:56 2018 @@ -80,11 +80,27 @@ bool IndexingContext::handleReference(co RefE, RefD, DC); } +static void reportModuleReferences(const Module *Mod, + ArrayRef IdLocs, + const ImportDecl *ImportD, + IndexDataConsumer ) { + if (!Mod) +return; + reportModuleReferences(Mod->Parent, IdLocs.drop_back(), ImportD, +
r341324 - Add header guards to some headers that are missing them
Author: akirtzidis Date: Mon Sep 3 09:26:36 2018 New Revision: 341324 URL: http://llvm.org/viewvc/llvm-project?rev=341324=rev Log: Add header guards to some headers that are missing them Modified: cfe/trunk/include/clang/AST/ODRHash.h cfe/trunk/lib/CodeGen/MacroPPCallbacks.h cfe/trunk/unittests/Rename/ClangRenameTest.h Modified: cfe/trunk/include/clang/AST/ODRHash.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ODRHash.h?rev=341324=341323=341324=diff == --- cfe/trunk/include/clang/AST/ODRHash.h (original) +++ cfe/trunk/include/clang/AST/ODRHash.h Mon Sep 3 09:26:36 2018 @@ -13,6 +13,9 @@ /// //===--===// +#ifndef LLVM_CLANG_AST_ODRHASH_H +#define LLVM_CLANG_AST_ODRHASH_H + #include "clang/AST/DeclarationName.h" #include "clang/AST/Type.h" #include "clang/AST/TemplateBase.h" @@ -91,3 +94,5 @@ public: }; } // end namespace clang + +#endif Modified: cfe/trunk/lib/CodeGen/MacroPPCallbacks.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MacroPPCallbacks.h?rev=341324=341323=341324=diff == --- cfe/trunk/lib/CodeGen/MacroPPCallbacks.h (original) +++ cfe/trunk/lib/CodeGen/MacroPPCallbacks.h Mon Sep 3 09:26:36 2018 @@ -11,6 +11,9 @@ // //===--===// +#ifndef LLVM_CLANG_LIB_CODEGEN_MACROPPCALLBACKS_H +#define LLVM_CLANG_LIB_CODEGEN_MACROPPCALLBACKS_H + #include "clang/Lex/PPCallbacks.h" namespace llvm { @@ -116,3 +119,5 @@ public: }; } // end namespace clang + +#endif Modified: cfe/trunk/unittests/Rename/ClangRenameTest.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Rename/ClangRenameTest.h?rev=341324=341323=341324=diff == --- cfe/trunk/unittests/Rename/ClangRenameTest.h (original) +++ cfe/trunk/unittests/Rename/ClangRenameTest.h Mon Sep 3 09:26:36 2018 @@ -7,6 +7,9 @@ // //===--===// +#ifndef LLVM_CLANG_UNITTESTS_RENAME_CLANGRENAMETEST_H +#define LLVM_CLANG_UNITTESTS_RENAME_CLANGRENAMETEST_H + #include "unittests/Tooling/RewriterTestContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/FileManager.h" @@ -110,3 +113,5 @@ protected: } // namespace test } // namespace clang_rename } // namesdpace clang + +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r341161 - [clang-move] Explicitly ignore implicit UsingDirectiveDecls instead of depending on them missing source locations
Author: akirtzidis Date: Thu Aug 30 20:51:33 2018 New Revision: 341161 URL: http://llvm.org/viewvc/llvm-project?rev=341161=rev Log: [clang-move] Explicitly ignore implicit UsingDirectiveDecls instead of depending on them missing source locations This is adjustment to allow the logic to work even if implicit UsingDirectiveDecls get actual source locations. There should be no functionality change. Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp Modified: clang-tools-extra/trunk/clang-move/ClangMove.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/ClangMove.cpp?rev=341161=341160=341161=diff == --- clang-tools-extra/trunk/clang-move/ClangMove.cpp (original) +++ clang-tools-extra/trunk/clang-move/ClangMove.cpp Thu Aug 30 20:51:33 2018 @@ -558,7 +558,8 @@ void ClangMoveTool::registerMatchers(ast // namespace, these decls are always copied to new.h/cc. Those in classes, // functions are covered in other matchers. Finder->addMatcher(namedDecl(anyOf(usingDecl(IsOldCCTopLevelDecl), - usingDirectiveDecl(IsOldCCTopLevelDecl), + usingDirectiveDecl(unless(isImplicit()), +IsOldCCTopLevelDecl), typeAliasDecl(IsOldCCTopLevelDecl)), notInMacro()) .bind("using_decl"), ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r340696 - [index] Introduce 'ProtocolInterface' as part of SymbolPropertySet
Author: akirtzidis Date: Sat Aug 25 23:27:23 2018 New Revision: 340696 URL: http://llvm.org/viewvc/llvm-project?rev=340696=rev Log: [index] Introduce 'ProtocolInterface' as part of SymbolPropertySet This is useful to directly infer that a method or property is from a protocol interface at the point of the symbol occurrences. Modified: cfe/trunk/include/clang/Index/IndexSymbol.h cfe/trunk/lib/Index/IndexSymbol.cpp cfe/trunk/test/Index/Core/external-source-symbol-attr.m cfe/trunk/test/Index/Core/index-source.m Modified: cfe/trunk/include/clang/Index/IndexSymbol.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexSymbol.h?rev=340696=340695=340696=diff == --- cfe/trunk/include/clang/Index/IndexSymbol.h (original) +++ cfe/trunk/include/clang/Index/IndexSymbol.h Sat Aug 25 23:27:23 2018 @@ -75,7 +75,7 @@ enum class SymbolSubKind : uint8_t { UsingValue, }; -typedef uint8_t SymbolPropertySet; +typedef uint16_t SymbolPropertySet; /// Set of properties that provide additional info about a symbol. enum class SymbolProperty : SymbolPropertySet { Generic = 1 << 0, @@ -86,8 +86,10 @@ enum class SymbolProperty : SymbolProper IBOutletCollection= 1 << 5, GKInspectable = 1 << 6, Local = 1 << 7, + /// Symbol is part of a protocol interface. + ProtocolInterface = 1 << 8, }; -static const unsigned SymbolPropertyBitNum = 8; +static const unsigned SymbolPropertyBitNum = 9; /// Set of roles that are attributed to symbol occurrences. /// Modified: cfe/trunk/lib/Index/IndexSymbol.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexSymbol.cpp?rev=340696=340695=340696=diff == --- cfe/trunk/lib/Index/IndexSymbol.cpp (original) +++ cfe/trunk/lib/Index/IndexSymbol.cpp Sat Aug 25 23:27:23 2018 @@ -96,6 +96,9 @@ SymbolInfo index::getSymbolInfo(const De if (isFunctionLocalSymbol(D)) { Info.Properties |= (SymbolPropertySet)SymbolProperty::Local; } + if (isa(D->getDeclContext())) { +Info.Properties |= (SymbolPropertySet)SymbolProperty::ProtocolInterface; + } if (const TagDecl *TD = dyn_cast(D)) { switch (TD->getTagKind()) { @@ -519,6 +522,7 @@ void index::applyForEachSymbolProperty(S APPLY_FOR_PROPERTY(IBOutletCollection); APPLY_FOR_PROPERTY(GKInspectable); APPLY_FOR_PROPERTY(Local); + APPLY_FOR_PROPERTY(ProtocolInterface); #undef APPLY_FOR_PROPERTY } @@ -539,6 +543,7 @@ void index::printSymbolProperties(Symbol case SymbolProperty::IBOutletCollection: OS << "IBColl"; break; case SymbolProperty::GKInspectable: OS << "GKI"; break; case SymbolProperty::Local: OS << "local"; break; +case SymbolProperty::ProtocolInterface: OS << "protocol"; break; } }); } Modified: cfe/trunk/test/Index/Core/external-source-symbol-attr.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/external-source-symbol-attr.m?rev=340696=340695=340696=diff == --- cfe/trunk/test/Index/Core/external-source-symbol-attr.m (original) +++ cfe/trunk/test/Index/Core/external-source-symbol-attr.m Sat Aug 25 23:27:23 2018 @@ -87,7 +87,7 @@ void test2(I3 *i3, id prot2, S [i3 meth2]; // CHECK: [[@LINE-1]]:7 | instance-method/Swift | meth2 | c:@CM@modname@objc(cs)I3(im)meth2 | [prot2 meth]; - // CHECK: [[@LINE-1]]:10 | instance-method/Swift | meth | c:@M@modname@objc(pl)ExtProt2(im)meth | + // CHECK: [[@LINE-1]]:10 | instance-method(protocol)/Swift | meth | c:@M@modname@objc(pl)ExtProt2(im)meth | some = SomeEnumFirst; // CHECK: [[@LINE-1]]:10 | enumerator/Swift | SomeEnumFirst | c:@M@modname@E@SomeEnum@SomeEnumFirst | } Modified: cfe/trunk/test/Index/Core/index-source.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.m?rev=340696=340695=340696=diff == --- cfe/trunk/test/Index/Core/index-source.m (original) +++ cfe/trunk/test/Index/Core/index-source.m Sat Aug 25 23:27:23 2018 @@ -474,12 +474,12 @@ void testImplicitProperties(ImplicitProp @end @protocol Prot3 // CHECK: [[@LINE]]:11 | protocol/ObjC | Prot3 | [[PROT3_USR:.*]] | | Decl | --(void)meth; +-(void)meth; // CHECK: [[@LINE]]:8 | instance-method(protocol)/ObjC | meth | [[PROT3_meth_USR:.*]] | -[Prot3 meth] | Decl,Dyn,RelChild | @end void test_rec1() { id o1; - [o1 meth]; // CHECK: [[@LINE]]:7 | instance-method/ObjC | meth | {{.*}} | Ref,Call,Dyn,RelRec,RelCall,RelCont | rel: 3 + [o1 meth]; // CHECK: [[@LINE]]:7 | instance-method(protocol)/ObjC | meth | [[PROT3_meth_USR]] | {{.*}} | Ref,Call,Dyn,RelRec,RelCall,RelCont | rel: 3 // CHECK-NEXT: RelCall,RelCont | test_rec1 | // CHECK-NEXT:
r340109 - [index] For an ObjC message call, also record as receivers the protocols if they are present in the ObjC type
Author: akirtzidis Date: Fri Aug 17 16:50:59 2018 New Revision: 340109 URL: http://llvm.org/viewvc/llvm-project?rev=340109=rev Log: [index] For an ObjC message call, also record as receivers the protocols if they are present in the ObjC type Modified: cfe/trunk/lib/Index/IndexBody.cpp cfe/trunk/test/Index/Core/index-source.m Modified: cfe/trunk/lib/Index/IndexBody.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexBody.cpp?rev=340109=340108=340109=diff == --- cfe/trunk/lib/Index/IndexBody.cpp (original) +++ cfe/trunk/lib/Index/IndexBody.cpp Fri Aug 17 16:50:59 2018 @@ -259,8 +259,24 @@ public: if (isDynamic(E)) { Roles |= (unsigned)SymbolRole::Dynamic; -if (auto *RecD = E->getReceiverInterface()) - Relations.emplace_back((unsigned)SymbolRole::RelationReceivedBy, RecD); + +auto addReceivers = [&](const ObjCObjectType *Ty) { + if (!Ty) +return; + if (const auto *clsD = Ty->getInterface()) { +Relations.emplace_back((unsigned)SymbolRole::RelationReceivedBy, + clsD); + } + for (const auto *protD : Ty->quals()) { +Relations.emplace_back((unsigned)SymbolRole::RelationReceivedBy, + protD); + } +}; +QualType recT = E->getReceiverType(); +if (const auto *Ptr = recT->getAs()) + addReceivers(Ptr->getObjectType()); +else + addReceivers(recT->getAs()); } return IndexCtx.handleReference(MD, E->getSelectorStartLoc(), Modified: cfe/trunk/test/Index/Core/index-source.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.m?rev=340109=340108=340109=diff == --- cfe/trunk/test/Index/Core/index-source.m (original) +++ cfe/trunk/test/Index/Core/index-source.m Fri Aug 17 16:50:59 2018 @@ -2,7 +2,7 @@ // RUN: c-index-test core -print-source-symbols -include-locals -- %s -target x86_64-apple-macosx10.7 | FileCheck -check-prefix=LOCAL %s @interface Base -// CHECK: [[@LINE-1]]:12 | class/ObjC | Base | c:objc(cs)Base | _OBJC_CLASS_$_Base | Decl | rel: 0 +// CHECK: [[@LINE-1]]:12 | class/ObjC | Base | [[BASE_USR:.*]] | _OBJC_CLASS_$_Base | Decl | rel: 0 -(void)meth; // CHECK: [[@LINE-1]]:8 | instance-method/ObjC | meth | c:objc(cs)Base(im)meth | -[Base meth] | Decl,Dyn,RelChild | rel: 1 // CHECK-NEXT: RelChild | Base | c:objc(cs)Base @@ -60,7 +60,7 @@ void goo(Base *b) { Base *f = (Base *) 2; } -// CHECK: [[@LINE+1]]:11 | protocol/ObjC | Prot1 | c:objc(pl)Prot1 | | Decl | rel: 0 +// CHECK: [[@LINE+1]]:11 | protocol/ObjC | Prot1 | [[PROT1_USR:.*]] | | Decl | rel: 0 @protocol Prot1 @end @@ -472,3 +472,21 @@ void testImplicitProperties(ImplicitProp } @end + +@protocol Prot3 // CHECK: [[@LINE]]:11 | protocol/ObjC | Prot3 | [[PROT3_USR:.*]] | | Decl | +-(void)meth; +@end + +void test_rec1() { + id o1; + [o1 meth]; // CHECK: [[@LINE]]:7 | instance-method/ObjC | meth | {{.*}} | Ref,Call,Dyn,RelRec,RelCall,RelCont | rel: 3 +// CHECK-NEXT: RelCall,RelCont | test_rec1 | +// CHECK-NEXT: RelRec | Prot3 | [[PROT3_USR]] +// CHECK-NEXT: RelRec | Prot1 | [[PROT1_USR]] + Base *o2; + [o2 meth]; // CHECK: [[@LINE]]:7 | instance-method/ObjC | meth | {{.*}} | Ref,Call,Dyn,RelRec,RelCall,RelCont | rel: 4 +// CHECK-NEXT: RelCall,RelCont | test_rec1 | +// CHECK-NEXT: RelRec | Base | [[BASE_USR]] +// CHECK-NEXT: RelRec | Prot3 | [[PROT3_USR]] +// CHECK-NEXT: RelRec | Prot1 | [[PROT1_USR]] +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r323549 - [index] Fix crash when indexing a C++14 PCH/module related to TemplateTemplateParmDecls of alias templates
Author: akirtzidis Date: Fri Jan 26 11:26:12 2018 New Revision: 323549 URL: http://llvm.org/viewvc/llvm-project?rev=323549=rev Log: [index] Fix crash when indexing a C++14 PCH/module related to TemplateTemplateParmDecls of alias templates TemplateTemplateParmDecls of alias templates ended-up serialized as 'file-level decls' which was causing a crash while trying to index a PCH/module file that contained them. Commit makes sure TemplateTemplateParmDecls are not recorded as such kind of decls. Fixes crash of rdar://36608297 Differential Revision: https://reviews.llvm.org/D42588 Added: cfe/trunk/test/Index/Core/index-pch.cpp Modified: cfe/trunk/lib/Index/IndexDecl.cpp cfe/trunk/lib/Serialization/ASTWriter.cpp Modified: cfe/trunk/lib/Index/IndexDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=323549=323548=323549=diff == --- cfe/trunk/lib/Index/IndexDecl.cpp (original) +++ cfe/trunk/lib/Index/IndexDecl.cpp Fri Jan 26 11:26:12 2018 @@ -664,8 +664,11 @@ public: bool VisitTemplateDecl(const TemplateDecl *D) { -// Index the default values for the template parameters. const NamedDecl *Parent = D->getTemplatedDecl(); +if (!Parent) + return true; + +// Index the default values for the template parameters. if (D->getTemplateParameters() && shouldIndexTemplateParameterDefaultValue(Parent)) { const TemplateParameterList *Params = D->getTemplateParameters(); @@ -684,7 +687,7 @@ public: } } -return Visit(D->getTemplatedDecl()); +return Visit(Parent); } bool VisitFriendDecl(const FriendDecl *D) { Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=323549=323548=323549=diff == --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Jan 26 11:26:12 2018 @@ -5537,7 +5537,9 @@ void ASTWriter::associateDeclWithFile(co return; // FIXME: ParmVarDecls that are part of a function type of a parameter of // a function/objc method, should not have TU as lexical context. - if (isa(D)) + // TemplateTemplateParmDecls that are part of an alias template, should not + // have TU as lexical context. + if (isa(D) || isa(D)) return; SourceManager = Context->getSourceManager(); Added: cfe/trunk/test/Index/Core/index-pch.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-pch.cpp?rev=323549=auto == --- cfe/trunk/test/Index/Core/index-pch.cpp (added) +++ cfe/trunk/test/Index/Core/index-pch.cpp Fri Jan 26 11:26:12 2018 @@ -0,0 +1,17 @@ +// RUN: c-index-test core -print-source-symbols -- %s -std=c++14 | FileCheck %s +// RUN: %clang_cc1 -emit-pch %s -std=c++14 -o %t.pch +// RUN: c-index-test core -print-source-symbols -module-file %t.pch | FileCheck %s + +// CHECK: [[@LINE+2]]:8 | struct(Gen)/C++ | DETECTOR | [[DETECTOR_USR:.*]] | {{.*}} | Def | rel: 0 +template class _Op, class... _Args> +struct DETECTOR { + using value_t = int; +}; + +struct nonesuch {}; + +// CHECK: [[@LINE+4]]:9 | type-alias/C++ | is_detected +// CHECK: [[@LINE+3]]:32 | struct(Gen)/C++ | DETECTOR | [[DETECTOR_USR]] | {{.*}} | Ref,RelCont | rel: 1 +// CHECK-NEXT: RelCont | is_detected +template + using is_detected = typename DETECTOR::value_t; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r310933 - [index] Update indexing to handle CXXDeductionGuideDecls properly
Author: akirtzidis Date: Tue Aug 15 10:20:37 2017 New Revision: 310933 URL: http://llvm.org/viewvc/llvm-project?rev=310933=rev Log: [index] Update indexing to handle CXXDeductionGuideDecls properly CXXDeductionGuideDecls can't be referenced so there's no need to output a symbol occurrence for them. Also handle DeducedTemplateSpecializationTypeLocs in the TypeIndexer so we don't miss the symbol occurrences of the corresponding template decls. Patch by Nathan Hawes! Differential Revision: https://reviews.llvm.org/D36641 Modified: cfe/trunk/lib/Index/IndexDecl.cpp cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp cfe/trunk/lib/Index/IndexingContext.cpp cfe/trunk/test/Index/Core/index-source.cpp Modified: cfe/trunk/lib/Index/IndexDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=310933=310932=310933=diff == --- cfe/trunk/lib/Index/IndexDecl.cpp (original) +++ cfe/trunk/lib/Index/IndexDecl.cpp Tue Aug 15 10:20:37 2017 @@ -267,6 +267,10 @@ public: TypeNameInfo->getTypeLoc().getLocStart(), Dtor->getParent(), Dtor->getDeclContext()); } +} else if (const auto *Guide = dyn_cast(D)) { + IndexCtx.handleReference(Guide->getDeducedTemplate()->getTemplatedDecl(), + Guide->getLocation(), Guide, + Guide->getDeclContext()); } // Template specialization arguments. if (const ASTTemplateArgumentListInfo *TemplateArgInfo = Modified: cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp?rev=310933=310932=310933=diff == --- cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp (original) +++ cfe/trunk/lib/Index/IndexTypeSourceInfo.cpp Tue Aug 15 10:20:37 2017 @@ -126,8 +126,9 @@ public: return true; } - bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) { -if (const TemplateSpecializationType *T = TL.getTypePtr()) { + template + bool HandleTemplateSpecializationTypeLoc(TypeLocType TL) { +if (const auto *T = TL.getTypePtr()) { if (IndexCtx.shouldIndexImplicitTemplateInsts()) { if (CXXRecordDecl *RD = T->getAsCXXRecordDecl()) IndexCtx.handleReference(RD, TL.getTemplateNameLoc(), @@ -141,6 +142,14 @@ public: return true; } + bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) { +return HandleTemplateSpecializationTypeLoc(TL); + } + + bool VisitDeducedTemplateSpecializationTypeLoc(DeducedTemplateSpecializationTypeLoc TL) { +return HandleTemplateSpecializationTypeLoc(TL); + } + bool VisitDependentNameTypeLoc(DependentNameTypeLoc TL) { const DependentNameType *DNT = TL.getTypePtr(); const NestedNameSpecifier *NNS = DNT->getQualifier(); Modified: cfe/trunk/lib/Index/IndexingContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingContext.cpp?rev=310933=310932=310933=diff == --- cfe/trunk/lib/Index/IndexingContext.cpp (original) +++ cfe/trunk/lib/Index/IndexingContext.cpp Tue Aug 15 10:20:37 2017 @@ -231,8 +231,8 @@ static bool isDeclADefinition(const Decl /// Whether the given NamedDecl should be skipped because it has no name. static bool shouldSkipNamelessDecl(const NamedDecl *ND) { - return ND->getDeclName().isEmpty() && !isa(ND) && - !isa(ND); + return (ND->getDeclName().isEmpty() && !isa(ND) && + !isa(ND)) || isa(ND); } static const Decl *adjustParent(const Decl *Parent) { Modified: cfe/trunk/test/Index/Core/index-source.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=310933=310932=310933=diff == --- cfe/trunk/test/Index/Core/index-source.cpp (original) +++ cfe/trunk/test/Index/Core/index-source.cpp Tue Aug 15 10:20:37 2017 @@ -497,6 +497,19 @@ void localStructuredBindingAndRef() { } +template +struct Guided { T t; }; +// CHECK: [[@LINE-1]]:8 | struct(Gen)/C++ | Guided | c:@ST>1#T@Guided | | Def | rel: 0 +// CHECK-NEXT: [[@LINE-2]]:19 | field/C++ | t | c:@ST>1#T@Guided@FI@t | | Def,RelChild | rel: 1 +// CHECK-NEXT: RelChild | Guided | c:@ST>1#T@Guided +Guided(double) -> Guided; +// CHECK: [[@LINE-1]]:19 | struct(Gen)/C++ | Guided | c:@ST>1#T@Guided | | Ref | rel: 0 +// CHECK-NEXT: [[@LINE-2]]:1 | struct(Gen)/C++ | Guided | c:@ST>1#T@Guided | | Ref | rel: 0 +auto guided = Guided{1.0}; +// CHECK: [[@LINE-1]]:6 | variable/C | guided | c:@guided | _guided | Def | rel: 0 +// CHECK-NEXT: [[@LINE-2]]:15 | struct(Gen)/C++ | Guided | c:@ST>1#T@Guided | | Ref,RelCont | rel: 1 +// CHECK-NEXT: RelCont | guided | c:@guided +
r305940 - [preprocessor] Fix assertion hit when 'SingleFileParseMode' option is enabled and #if with an undefined identifier and without #else
Author: akirtzidis Date: Wed Jun 21 13:52:44 2017 New Revision: 305940 URL: http://llvm.org/viewvc/llvm-project?rev=305940=rev Log: [preprocessor] Fix assertion hit when 'SingleFileParseMode' option is enabled and #if with an undefined identifier and without #else 'HandleEndifDirective' asserts that 'WasSkipping' is false, so switch to using 'FoundNonSkip' as the hint for 'SingleFileParseMode' to keep going with parsing. Modified: cfe/trunk/lib/Lex/PPDirectives.cpp cfe/trunk/test/Index/single-file-parse.m Modified: cfe/trunk/lib/Lex/PPDirectives.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=305940=305939=305940=diff == --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) +++ cfe/trunk/lib/Lex/PPDirectives.cpp Wed Jun 21 13:52:44 2017 @@ -2658,7 +2658,7 @@ void Preprocessor::HandleIfdefDirective( // In 'single-file-parse mode' undefined identifiers trigger parsing of all // the directive blocks. CurPPLexer->pushConditionalLevel(DirectiveTok.getLocation(), - /*wasskip*/true, /*foundnonskip*/false, + /*wasskip*/false, /*foundnonskip*/false, /*foundelse*/false); } else if (!MI == isIfndef) { // Yes, remember that we are inside a conditional, then lex the next token. @@ -2705,7 +2705,7 @@ void Preprocessor::HandleIfDirective(Tok if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) { // In 'single-file-parse mode' undefined identifiers trigger parsing of all // the directive blocks. -CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/true, +CurPPLexer->pushConditionalLevel(IfToken.getLocation(), /*wasskip*/false, /*foundnonskip*/false, /*foundelse*/false); } else if (ConditionalTrue) { // Yes, remember that we are inside a conditional, then lex the next token. @@ -2768,11 +2768,11 @@ void Preprocessor::HandleElseDirective(T if (Callbacks) Callbacks->Else(Result.getLocation(), CI.IfLoc); - if (PPOpts->SingleFileParseMode && CI.WasSkipping) { + if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) { // In 'single-file-parse mode' undefined identifiers trigger parsing of all // the directive blocks. CurPPLexer->pushConditionalLevel(CI.IfLoc, /*wasskip*/false, - /*foundnonskip*/true, /*foundelse*/true); + /*foundnonskip*/false, /*foundelse*/true); return; } @@ -2811,10 +2811,10 @@ void Preprocessor::HandleElifDirective(T SourceRange(ConditionalBegin, ConditionalEnd), PPCallbacks::CVK_NotEvaluated, CI.IfLoc); - if (PPOpts->SingleFileParseMode && CI.WasSkipping) { + if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) { // In 'single-file-parse mode' undefined identifiers trigger parsing of all // the directive blocks. -CurPPLexer->pushConditionalLevel(ElifToken.getLocation(), /*wasskip*/true, +CurPPLexer->pushConditionalLevel(ElifToken.getLocation(), /*wasskip*/false, /*foundnonskip*/false, /*foundelse*/false); return; } Modified: cfe/trunk/test/Index/single-file-parse.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/single-file-parse.m?rev=305940=305939=305940=diff == --- cfe/trunk/test/Index/single-file-parse.m (original) +++ cfe/trunk/test/Index/single-file-parse.m Wed Jun 21 13:52:44 2017 @@ -109,3 +109,13 @@ // CHECK: [[@LINE+1]]:12: ObjCInterfaceDecl=Test28 @interface Test28 @end #endif + +#if SOMETHING_NOT_DEFINED +// CHECK: [[@LINE+1]]:12: ObjCInterfaceDecl=Test29 +@interface Test29 @end +#endif + +#ifdef SOMETHING_NOT_DEFINED +// CHECK: [[@LINE+1]]:12: ObjCInterfaceDecl=Test30 +@interface Test30 @end +#endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits