From 581f4ac5bb433ff350407fcb5cfcb6d9d7345372 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Wed, 6 Sep 2017 14:23:21 -0700 Subject: [PATCH] Soft-deprecate -fapinotes-cache-path; use -fmodules-cache-path instead. (#119) When Clang decided to change the default module cache path to include the current user's name, the API notes path didn't change. Rather than duplicate that logic, just put the API notes cache in with the module cache by default. We still support -fapinotes-cache-path in case anyone's using it, but from now on -fmodules-cache-path should be sufficient. (This is reasonable because API notes are currently linked to modules anyway.) https://bugs.swift.org/browse/SR-5806 --- .../clang/Basic/DiagnosticFrontendKinds.td | 2 +- include/clang/Driver/Options.td | 2 +- lib/Driver/ToolChains/Clang.cpp | 22 +++---------------- lib/Frontend/CompilerInvocation.cpp | 11 +++++++--- test/APINotes/cache.m | 16 ++++++++++---- 5 files changed, 25 insertions(+), 28 deletions(-) diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td b/include/clang/Basic/DiagnosticFrontendKinds.td index 446eb41ea02..3354048ef76 100644 --- a/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/include/clang/Basic/DiagnosticFrontendKinds.td @@ -230,7 +230,7 @@ def err_invalid_vfs_overlay : Error< "invalid virtual filesystem overlay file '%0'">, DefaultFatal; def err_no_apinotes_cache_path : Error< - "-fapinotes was provided without -fapinotes-cache-path=">, + "-fapinotes was provided without -fmodules-cache-path=">, DefaultFatal; def warn_option_invalid_ocl_version : Warning< diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td index 0323f7e0a56..cfea4226d8c 100644 --- a/include/clang/Driver/Options.td +++ b/include/clang/Driver/Options.td @@ -719,7 +719,7 @@ def fno_apinotes_modules : Flag<["-"], "fno-apinotes-modules">, Group, HelpText<"Disable module-based external API notes support">; def fapinotes_cache_path : Joined<["-"], "fapinotes-cache-path=">, Group, Flags<[DriverOption, CC1Option]>, MetaVarName<"">, - HelpText<"Specify the API notes cache path">; + HelpText<"Specify the API notes cache path (defaults to -fmodules-cache-path)">; def fapinotes_swift_version : Joined<["-"], "fapinotes-swift-version=">, Group, Flags<[CC1Option]>, MetaVarName<"">, HelpText<"Specify the Swift version to use when filtering API notes">; diff --git a/lib/Driver/ToolChains/Clang.cpp b/lib/Driver/ToolChains/Clang.cpp index ce662bc4f6a..120bba26887 100644 --- a/lib/Driver/ToolChains/Clang.cpp +++ b/lib/Driver/ToolChains/Clang.cpp @@ -3985,28 +3985,12 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, options::OPT_fno_apinotes_modules, false)) CmdArgs.push_back("-fapinotes-modules"); - SmallString<128> APINotesCachePath; if (Arg *A = Args.getLastArg(options::OPT_fapinotes_cache_path)) { - APINotesCachePath = A->getValue(); + SmallString<128> APINotesCachePath{"-fapinotes-cache-path="}; + APINotesCachePath += A->getValue(); + CmdArgs.push_back(Args.MakeArgString(APINotesCachePath)); } - if (C.isForDiagnostics()) { - // When generating crash reports, we want to emit the API notes along with - // the reproduction sources, so we ignore any provided API notes path. - APINotesCachePath = Output.getFilename(); - llvm::sys::path::replace_extension(APINotesCachePath, ".cache"); - llvm::sys::path::append(APINotesCachePath, "apinotes"); - } else if (APINotesCachePath.empty()) { - // No API notes path was provided: use the default. - llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, - APINotesCachePath); - llvm::sys::path::append(APINotesCachePath, "org.llvm.clang"); - llvm::sys::path::append(APINotesCachePath, "APINotesCache"); - } - const char Arg[] = "-fapinotes-cache-path="; - APINotesCachePath.insert(APINotesCachePath.begin(), Arg, Arg + strlen(Arg)); - CmdArgs.push_back(Args.MakeArgString(APINotesCachePath)); - Args.AddLastArg(CmdArgs, options::OPT_fapinotes_swift_version); } diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index d1b3beaf755..dc8d6688e85 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -2731,11 +2731,16 @@ bool CompilerInvocation::CreateFromArgs(CompilerInvocation &Res, if (Res.getFrontendOpts().ProgramAction == frontend::RewriteObjC) LangOpts.ObjCExceptions = 1; - // -fapinotes and -fapinotes-modules requires -fapinotes-cache-path=. + // -fapinotes and -fapinotes-modules requires -fmodules-cache-path=. if ((LangOpts.APINotes || LangOpts.APINotesModules) && Res.getFileSystemOpts().APINotesCachePath.empty()) { - Diags.Report(diag::err_no_apinotes_cache_path); - Success = false; + if (!Res.getHeaderSearchOpts().ModuleCachePath.empty()) { + Res.getFileSystemOpts().APINotesCachePath = + Res.getHeaderSearchOpts().ModuleCachePath; + } else { + Diags.Report(diag::err_no_apinotes_cache_path); + Success = false; + } } } diff --git a/test/APINotes/cache.m b/test/APINotes/cache.m index b87bdf14fec..08a2f263a5c 100644 --- a/test/APINotes/cache.m +++ b/test/APINotes/cache.m @@ -1,4 +1,4 @@ -// RUN: rm -rf %t/APINotesCache +// RUN: rm -rf %t // RUN: %clang_cc1 -fapinotes -fapinotes-modules -fapinotes-cache-path=%t/APINotesCache -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -verify // Check for the presence of the cached compiled form. @@ -6,11 +6,19 @@ // RUN: ls %t/APINotesCache | grep "SomeKit-.*.apinotesc" // Run test again to ensure that caching doesn't cause problems. -// RUN: %clang_cc1 -fapinotes -fapinotes-modules -fapinotes-cache-path=%t/APINotesCache -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -verify +// RUN: %clang_cc1 -fapinotes -fapinotes-modules -fapinotes-cache-path=%t/APINotesCache -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -verify + +// Check that the default path is taken from -fmodules-cache-path. +// RUN: %clang_cc1 -fapinotes -fapinotes-modules -fmodules-cache-path=%t/ModuleCache -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -verify +// RUN: ls %t/ModuleCache | grep "APINotes-.*.apinotesc" +// RUN: ls %t/ModuleCache | grep "SomeKit-.*.apinotesc" + +// RUN: not %clang_cc1 -fapinotes -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s 2>&1 | FileCheck --check-prefix=CHECK-NO-CACHE %s +// CHECK-NO-CACHE: error: -fapinotes was provided without -fmodules-cache-path -// Check that the driver provides a default -fapinotes-cache-path= +// Check that the driver does not provide a default -fapinotes-cache-path=. // RUN: %clang -fsyntax-only -fapinotes -fapinotes-modules -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -### 2>&1 | FileCheck --check-prefix=CHECK-DEFAULT-PATH %s -// CHECK-DEFAULT-PATH: -fapinotes-cache-path={{.*}}org.llvm.clang/APINotesCache +// CHECK-DEFAULT-PATH-NOT: -fapinotes-cache-path // Check that the driver passes through a provided -fapinotes-cache-path= // RUN: %clang -fsyntax-only -fapinotes -fapinotes-modules -fapinotes-cache-path=/wobble -I %S/Inputs/Headers -F %S/Inputs/Frameworks %s -### 2>&1 | FileCheck --check-prefix=CHECK-PATH %s