vedgy updated this revision to Diff 495372.
vedgy edited the summary of this revision.
vedgy added a comment.
Address an old inline review comment
https://reviews.llvm.org/D139774#inline-1360634 and add a comment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143418/new/
https://reviews.llvm.org/D143418
Files:
clang-tools-extra/clangd/Preamble.cpp
clang/docs/ReleaseNotes.rst
clang/include/clang-c/Index.h
clang/include/clang/Frontend/ASTUnit.h
clang/include/clang/Frontend/PrecompiledPreamble.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/PrecompiledPreamble.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/CIndexer.h
clang/tools/libclang/libclang.map
clang/unittests/Frontend/ASTUnitTest.cpp
clang/unittests/libclang/LibclangTest.cpp
clang/unittests/libclang/TestUtils.h
Index: clang/unittests/libclang/TestUtils.h
===================================================================
--- clang/unittests/libclang/TestUtils.h
+++ clang/unittests/libclang/TestUtils.h
@@ -22,11 +22,11 @@
#include <vector>
class LibclangParseTest : public ::testing::Test {
- // std::greater<> to remove files before their parent dirs in TearDown().
- std::set<std::string, std::greater<>> Files;
typedef std::unique_ptr<std::string> fixed_addr_string;
std::map<fixed_addr_string, fixed_addr_string> UnsavedFileContents;
public:
+ // std::greater<> to remove files before their parent dirs in TearDown().
+ std::set<std::string, std::greater<>> FilesAndDirsToRemove;
std::string TestDir;
bool RemoveTestDirRecursivelyDuringTeardown = false;
CXIndex Index;
@@ -48,7 +48,7 @@
clang_disposeIndex(Index);
namespace fs = llvm::sys::fs;
- for (const std::string &Path : Files)
+ for (const std::string &Path : FilesAndDirsToRemove)
EXPECT_FALSE(fs::remove(Path, /*IgnoreNonExisting=*/false));
if (RemoveTestDirRecursivelyDuringTeardown)
EXPECT_FALSE(fs::remove_directories(TestDir, /*IgnoreErrors=*/false));
@@ -63,7 +63,7 @@
FileI != FileEnd; ++FileI) {
ASSERT_NE(*FileI, ".");
path::append(Path, *FileI);
- Files.emplace(Path.str());
+ FilesAndDirsToRemove.emplace(Path.str());
}
Filename = std::string(Path.str());
}
Index: clang/unittests/libclang/LibclangTest.cpp
===================================================================
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -15,6 +15,7 @@
#include "llvm/Support/raw_ostream.h"
#include "gtest/gtest.h"
#include "TestUtils.h"
+#include <cstring>
#include <fstream>
#include <functional>
#include <map>
@@ -355,6 +356,94 @@
clang_ModuleMapDescriptor_dispose(MMD);
}
+class LibclangTempDirTest : public LibclangParseTest {
+public:
+ std::string Main = "main.cpp";
+ std::string TempDir;
+
+ void SetUp() override {
+ LibclangParseTest::SetUp();
+ TUFlags |= CXTranslationUnit_CreatePreambleOnFirstParse;
+ // For some reason, the preamble is not created without '\n' before `int`.
+ WriteFile(Main, "\nint main() {}");
+
+ llvm::SmallString<128> TempDirBuffer(TestDir);
+ llvm::sys::path::append(TempDirBuffer, "temp_dir");
+ namespace fs = llvm::sys::fs;
+ ASSERT_FALSE(
+ fs::create_directory(TempDirBuffer, false, fs::perms::owner_all));
+
+ TempDir = static_cast<std::string>(TempDirBuffer);
+ FilesAndDirsToRemove.insert(TempDir);
+ }
+ void CheckPreamblesInTempDir(int PreambleCount) {
+ ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0,
+ nullptr, 0, TUFlags);
+
+ int FileCount = 0;
+
+ namespace fs = llvm::sys::fs;
+ std::error_code EC;
+ for (fs::directory_iterator File(TempDir, EC), FileEnd;
+ File != FileEnd && !EC; File.increment(EC)) {
+ ++FileCount;
+
+ EXPECT_EQ(File->type(), fs::file_type::regular_file);
+
+ const auto Filename = llvm::sys::path::filename(File->path());
+ EXPECT_EQ(Filename.size(), std::strlen("preamble-%%%%%%.pch"));
+ EXPECT_TRUE(Filename.startswith("preamble-"));
+ EXPECT_TRUE(Filename.endswith(".pch"));
+
+ const auto Status = File->status();
+ ASSERT_TRUE(Status);
+ if (false) {
+ // The permissions assertion below fails, because the .pch.tmp file is
+ // created with default permissions and replaces the .pch file along
+ // with its permissions. Therefore the permissions set in
+ // TempPCHFile::create() don't matter in the end.
+ EXPECT_EQ(Status->permissions(), fs::owner_read | fs::owner_write);
+ }
+ }
+
+ EXPECT_EQ(FileCount, PreambleCount);
+ }
+};
+
+TEST_F(LibclangTempDirTest, clang_CXIndex_setPreferredTempDirPath_notCalled) {
+ CheckPreamblesInTempDir(0);
+}
+
+TEST_F(LibclangTempDirTest, clang_CXIndex_setPreferredTempDirPath_toNull) {
+ clang_CXIndex_setPreferredTempDirPath(Index, nullptr);
+ CheckPreamblesInTempDir(0);
+}
+
+TEST_F(LibclangTempDirTest,
+ clang_CXIndex_setPreferredTempDirPath_toEmptyString) {
+ clang_CXIndex_setPreferredTempDirPath(Index, "");
+ CheckPreamblesInTempDir(0);
+}
+
+TEST_F(LibclangTempDirTest, clang_CXIndex_setPreferredTempDirPath_toTempDir) {
+ clang_CXIndex_setPreferredTempDirPath(Index, TempDir.c_str());
+ CheckPreamblesInTempDir(1);
+}
+
+TEST_F(LibclangTempDirTest,
+ clang_CXIndex_setPreferredTempDirPath_toTempDirThenNull) {
+ clang_CXIndex_setPreferredTempDirPath(Index, TempDir.c_str());
+ clang_CXIndex_setPreferredTempDirPath(Index, nullptr);
+ CheckPreamblesInTempDir(0);
+}
+
+TEST_F(LibclangTempDirTest,
+ clang_CXIndex_setPreferredTempDirPath_toRandomStringThenTempDir) {
+ clang_CXIndex_setPreferredTempDirPath(Index, "/tmp/randomString");
+ clang_CXIndex_setPreferredTempDirPath(Index, TempDir.c_str());
+ CheckPreamblesInTempDir(1);
+}
+
TEST_F(LibclangParseTest, AllSkippedRanges) {
std::string Header = "header.h", Main = "main.cpp";
WriteFile(Header,
Index: clang/unittests/Frontend/ASTUnitTest.cpp
===================================================================
--- clang/unittests/Frontend/ASTUnitTest.cpp
+++ clang/unittests/Frontend/ASTUnitTest.cpp
@@ -167,7 +167,7 @@
std::unique_ptr<clang::ASTUnit> ErrUnit;
ASTUnit *AST = ASTUnit::LoadFromCommandLine(
- &Args[0], &Args[4], PCHContainerOps, Diags, "", false,
+ &Args[0], &Args[4], PCHContainerOps, Diags, "", "", false,
CaptureDiagsKind::All, std::nullopt, true, 0, TU_Complete, false, false,
false, SkipFunctionBodiesScope::None, false, true, false, false,
std::nullopt, &ErrUnit, nullptr);
Index: clang/tools/libclang/libclang.map
===================================================================
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -419,6 +419,11 @@
clang_CXXMethod_isExplicit;
};
+LLVM_17 {
+ global:
+ clang_CXIndex_setPreferredTempDirPath;
+};
+
# Example of how to add a new symbol version entry. If you do add a new symbol
# version, please update the example to depend on the version you added.
# LLVM_X {
Index: clang/tools/libclang/CIndexer.h
===================================================================
--- clang/tools/libclang/CIndexer.h
+++ clang/tools/libclang/CIndexer.h
@@ -41,6 +41,7 @@
std::string ToolchainPath;
+ std::string PreferredTempDirPath;
std::string InvocationEmissionPath;
public:
@@ -77,6 +78,12 @@
StringRef getClangToolchainPath();
+ void setPreferredTempDirPath(StringRef Str) {
+ PreferredTempDirPath = Str.str();
+ }
+
+ StringRef getPreferredTempDirPath() const { return PreferredTempDirPath; }
+
void setInvocationEmissionPath(StringRef Str) {
InvocationEmissionPath = std::string(Str);
}
Index: clang/tools/libclang/CIndex.cpp
===================================================================
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3697,6 +3697,11 @@
return 0;
}
+void clang_CXIndex_setPreferredTempDirPath(CXIndex CIdx, const char *path) {
+ if (CIdx)
+ static_cast<CIndexer *>(CIdx)->setPreferredTempDirPath(path);
+}
+
void clang_CXIndex_setInvocationEmissionPathOption(CXIndex CIdx,
const char *Path) {
if (CIdx)
@@ -3895,8 +3900,8 @@
std::unique_ptr<ASTUnit> Unit(ASTUnit::LoadFromCommandLine(
Args->data(), Args->data() + Args->size(),
CXXIdx->getPCHContainerOperations(), Diags,
- CXXIdx->getClangResourcesPath(), CXXIdx->getOnlyLocalDecls(),
- CaptureDiagnostics, *RemappedFiles.get(),
+ CXXIdx->getClangResourcesPath(), CXXIdx->getPreferredTempDirPath(),
+ CXXIdx->getOnlyLocalDecls(), CaptureDiagnostics, *RemappedFiles.get(),
/*RemappedFilesKeepOriginalName=*/true, PrecompilePreambleAfterNParses,
TUKind, CacheCodeCompletionResults, IncludeBriefCommentsInCodeCompletion,
/*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse,
Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===================================================================
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -197,20 +197,32 @@
class TempPCHFile {
public:
// A main method used to construct TempPCHFile.
- static std::unique_ptr<TempPCHFile> create() {
+ static std::unique_ptr<TempPCHFile> create(StringRef StoragePath) {
// FIXME: This is a hack so that we can override the preamble file during
// crash-recovery testing, which is the only case where the preamble files
// are not necessarily cleaned up.
if (const char *TmpFile = ::getenv("CINDEXTEST_PREAMBLE_FILE"))
return std::unique_ptr<TempPCHFile>(new TempPCHFile(TmpFile));
- llvm::SmallString<64> File;
- // Using a version of createTemporaryFile with a file descriptor guarantees
+ llvm::SmallString<128> File;
+ // Using the versions of createTemporaryFile() and
+ // createUniqueFile() with a file descriptor guarantees
// that we would never get a race condition in a multi-threaded setting
// (i.e., multiple threads getting the same temporary path).
int FD;
- if (auto EC =
- llvm::sys::fs::createTemporaryFile("preamble", "pch", FD, File))
+ std::error_code EC;
+ if (StoragePath.empty())
+ EC = llvm::sys::fs::createTemporaryFile("preamble", "pch", FD, File);
+ else {
+ llvm::SmallString<128> TempPath = StoragePath;
+ // Use the same filename model as fs::createTemporaryFile().
+ llvm::sys::path::append(TempPath, "preamble-%%%%%%.pch");
+ namespace fs = llvm::sys::fs;
+ // Use the same owner-only file permissions as fs::createTemporaryFile().
+ EC = fs::createUniqueFile(TempPath, FD, File, fs::OF_None,
+ fs::owner_read | fs::owner_write);
+ }
+ if (EC)
return nullptr;
// We only needed to make sure the file exists, close the file right away.
llvm::sys::Process::SafelyCloseFileDescriptor(FD);
@@ -402,8 +414,8 @@
const llvm::MemoryBuffer *MainFileBuffer, PreambleBounds Bounds,
DiagnosticsEngine &Diagnostics,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
- std::shared_ptr<PCHContainerOperations> PCHContainerOps, bool StoreInMemory,
- PreambleCallbacks &Callbacks) {
+ std::shared_ptr<PCHContainerOperations> PCHContainerOps,
+ StringRef StoragePath, bool StoreInMemory, PreambleCallbacks &Callbacks) {
assert(VFS && "VFS is null");
auto PreambleInvocation = std::make_shared<CompilerInvocation>(Invocation);
@@ -418,7 +430,8 @@
} else {
// Create a temporary file for the precompiled preamble. In rare
// circumstances, this can fail.
- std::unique_ptr<TempPCHFile> PreamblePCHFile = TempPCHFile::create();
+ std::unique_ptr<TempPCHFile> PreamblePCHFile =
+ TempPCHFile::create(StoragePath);
if (!PreamblePCHFile)
return BuildPreambleError::CouldntCreateTempFile;
Storage = PCHStorage::file(std::move(PreamblePCHFile));
Index: clang/lib/Frontend/ASTUnit.cpp
===================================================================
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1393,7 +1393,8 @@
llvm::ErrorOr<PrecompiledPreamble> NewPreamble = PrecompiledPreamble::Build(
PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
- PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
+ PCHContainerOps, PreferredTempDirPath, /*StoreInMemory=*/false,
+ Callbacks);
PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies =
PreviousSkipFunctionBodies;
@@ -1737,12 +1738,13 @@
const char **ArgBegin, const char **ArgEnd,
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath,
- bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics,
- ArrayRef<RemappedFile> RemappedFiles, bool RemappedFilesKeepOriginalName,
- unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind,
- bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
- bool AllowPCHWithCompilerErrors, SkipFunctionBodiesScope SkipFunctionBodies,
- bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
+ StringRef PreferredTempDirPath, bool OnlyLocalDecls,
+ CaptureDiagsKind CaptureDiagnostics, ArrayRef<RemappedFile> RemappedFiles,
+ bool RemappedFilesKeepOriginalName, unsigned PrecompilePreambleAfterNParses,
+ TranslationUnitKind TUKind, bool CacheCodeCompletionResults,
+ bool IncludeBriefCommentsInCodeCompletion, bool AllowPCHWithCompilerErrors,
+ SkipFunctionBodiesScope SkipFunctionBodies, bool SingleFileParse,
+ bool UserFilesAreVolatile, bool ForSerialization,
bool RetainExcludedConditionalBlocks, std::optional<StringRef> ModuleFormat,
std::unique_ptr<ASTUnit> *ErrAST,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
@@ -1797,6 +1799,7 @@
VFS = llvm::vfs::getRealFileSystem();
VFS = createVFSFromCompilerInvocation(*CI, *Diags, VFS);
AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS);
+ AST->PreferredTempDirPath = PreferredTempDirPath;
AST->ModuleCache = new InMemoryModuleCache;
AST->OnlyLocalDecls = OnlyLocalDecls;
AST->CaptureDiagnostics = CaptureDiagnostics;
Index: clang/include/clang/Frontend/PrecompiledPreamble.h
===================================================================
--- clang/include/clang/Frontend/PrecompiledPreamble.h
+++ clang/include/clang/Frontend/PrecompiledPreamble.h
@@ -72,6 +72,10 @@
///
/// \param PCHContainerOps An instance of PCHContainerOperations.
///
+ /// \param StoragePath The path to a directory, in which to create a temporary
+ /// file to store PCH in. If empty, the default system temporary directory is
+ /// used. This parameter is ignored if \p StoreInMemory is true.
+ ///
/// \param StoreInMemory Store PCH in memory. If false, PCH will be stored in
/// a temporary file.
///
@@ -83,7 +87,8 @@
DiagnosticsEngine &Diagnostics,
IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
- bool StoreInMemory, PreambleCallbacks &Callbacks);
+ StringRef StoragePath, bool StoreInMemory,
+ PreambleCallbacks &Callbacks);
PrecompiledPreamble(PrecompiledPreamble &&);
PrecompiledPreamble &operator=(PrecompiledPreamble &&);
Index: clang/include/clang/Frontend/ASTUnit.h
===================================================================
--- clang/include/clang/Frontend/ASTUnit.h
+++ clang/include/clang/Frontend/ASTUnit.h
@@ -124,6 +124,7 @@
std::unique_ptr<ASTWriterData> WriterData;
FileSystemOptions FileSystemOpts;
+ std::string PreferredTempDirPath;
/// The AST consumer that received information about the translation
/// unit as it was parsed or loaded.
@@ -802,6 +803,9 @@
///
/// \param ResourceFilesPath - The path to the compiler resource files.
///
+ /// \param PreferredTempDirPath - The path to a directory, in which to create
+ /// temporary files. If empty, the default system temporary directory is used.
+ ///
/// \param ModuleFormat - If provided, uses the specific module format.
///
/// \param ErrAST - If non-null and parsing failed without any AST to return
@@ -820,7 +824,7 @@
const char **ArgBegin, const char **ArgEnd,
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
IntrusiveRefCntPtr<DiagnosticsEngine> Diags, StringRef ResourceFilesPath,
- bool OnlyLocalDecls = false,
+ StringRef PreferredTempDirPath = StringRef(), bool OnlyLocalDecls = false,
CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None,
ArrayRef<RemappedFile> RemappedFiles = std::nullopt,
bool RemappedFilesKeepOriginalName = true,
Index: clang/include/clang-c/Index.h
===================================================================
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -34,7 +34,7 @@
* compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
*/
#define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 63
+#define CINDEX_VERSION_MINOR 64
#define CINDEX_VERSION_ENCODE(major, minor) (((major)*10000) + ((minor)*1))
@@ -332,6 +332,24 @@
*/
CINDEX_LINKAGE unsigned clang_CXIndex_getGlobalOptions(CXIndex);
+/**
+ * Override the temporary directory path used by CXIndex.
+ *
+ * Libclang tries its best to create temporary files in the specified directory.
+ * But some files can still end up in the system temporary directory. Call this
+ * function right after clang_createIndex() to minimize the number of files
+ * created in the system temporary directory.
+ *
+ * Libclang does not create the directory at the specified path in the
+ * file system. Therefore it must exist, or the temporary files will be lost.
+ *
+ * Passing null or empty path cancels the overriding. Libclang tries its best to
+ * create temporary files in the default system temporary directory afterwards.
+ * But some temporary files can still be created in previous path overrides.
+ */
+CINDEX_LINKAGE void clang_CXIndex_setPreferredTempDirPath(CXIndex,
+ const char *path);
+
/**
* Sets the invocation emission path option in a CXIndex.
*
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -207,6 +207,8 @@
libclang
--------
+- Introduced the new function ``clang_CXIndex_setPreferredTempDirPath``,
+ which allows overriding the temporary directory path used by libclang.
Static Analyzer
---------------
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -545,7 +545,7 @@
auto BuiltPreamble = PrecompiledPreamble::Build(
CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine,
Stats ? TimedFS : StatCacheFS, std::make_shared<PCHContainerOperations>(),
- StoreInMemory, CapturedInfo);
+ /*StoragePath=*/StringRef(), StoreInMemory, CapturedInfo);
PreambleTimer.stopTimer();
// When building the AST for the main file, we do want the function
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
