Skip to content

Commit d768d73

Browse files
[CASOption] clang CASOption improvements
This addresses two issues in clang::CASOptions: * The equality comparison is wrong when plugin is used. * You need a clang diagnostics to create ObjectStore/ActionCache from CASOptions
1 parent 4a1b1cd commit d768d73

File tree

5 files changed

+61
-26
lines changed

5 files changed

+61
-26
lines changed

clang/include/clang/Basic/DiagnosticCASKinds.td

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,8 @@
88

99
let Component = "CAS" in {
1010

11-
def err_builtin_cas_cannot_be_initialized : Error<
12-
"CAS cannot be initialized from '%0' on disk (check -fcas-path): %1">,
13-
DefaultFatal;
14-
def err_plugin_cas_cannot_be_initialized : Error<
15-
"plugin CAS cannot be initialized from '%0': %1">,
16-
DefaultFatal;
17-
def err_builtin_actioncache_cannot_be_initialized : Error<
18-
"ActionCache cannot be initialized from '%0' on disk (check -fcas-path)">,
11+
def err_cas_cannot_be_initialized : Error<
12+
"CAS cannot be initialized from the specified '-fcas-*' options: %0">,
1913
DefaultFatal;
2014
def err_cas_cannot_parse_root_id : Error<
2115
"CAS cannot parse root-id '%0' specified by -fcas-fs">, DefaultFatal;

clang/include/clang/CAS/CASOptions.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#define LLVM_CLANG_CAS_CASOPTIONS_H
1616

1717
#include "llvm/ADT/SmallVector.h"
18+
#include "llvm/Support/Error.h"
1819
#include <string>
1920
#include <vector>
2021

@@ -63,7 +64,8 @@ class CASConfiguration {
6364

6465
friend bool operator==(const CASConfiguration &LHS,
6566
const CASConfiguration &RHS) {
66-
return LHS.CASPath == RHS.CASPath;
67+
return LHS.CASPath == RHS.CASPath && LHS.PluginPath == RHS.PluginPath &&
68+
LHS.PluginOptions == RHS.PluginOptions;
6769
}
6870
friend bool operator!=(const CASConfiguration &LHS,
6971
const CASConfiguration &RHS) {
@@ -101,6 +103,10 @@ class CASOptions : public CASConfiguration {
101103
getOrCreateDatabases(DiagnosticsEngine &Diags,
102104
bool CreateEmptyDBsOnFailure = false) const;
103105

106+
llvm::Expected<std::pair<std::shared_ptr<llvm::cas::ObjectStore>,
107+
std::shared_ptr<llvm::cas::ActionCache>>>
108+
getOrCreateDatabases() const;
109+
104110
/// Freeze CAS Configuration. Future calls will return the same
105111
/// CAS instance, even if the configuration changes again later.
106112
///
@@ -116,7 +122,7 @@ class CASOptions : public CASConfiguration {
116122

117123
private:
118124
/// Initialize Cached CAS and ActionCache.
119-
void initCache(DiagnosticsEngine &Diags) const;
125+
llvm::Error initCache() const;
120126

121127
struct CachedCAS {
122128
/// A cached CAS instance.

clang/lib/CAS/CASOptions.cpp

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "llvm/CAS/ActionCache.h"
1313
#include "llvm/CAS/BuiltinUnifiedCASDatabases.h"
1414
#include "llvm/CAS/ObjectStore.h"
15+
#include "llvm/Support/Error.h"
1516
#include "llvm/Support/Path.h"
1617

1718
using namespace clang;
@@ -31,20 +32,31 @@ CASOptions::getOrCreateDatabases(DiagnosticsEngine &Diags,
3132
if (Cache.Config.IsFrozen)
3233
return {Cache.CAS, Cache.AC};
3334

34-
initCache(Diags);
35+
if (auto E = initCache())
36+
Diags.Report(diag::err_cas_cannot_be_initialized) << toString(std::move(E));
37+
3538
if (!Cache.CAS && CreateEmptyDBsOnFailure)
3639
Cache.CAS = llvm::cas::createInMemoryCAS();
3740
if (!Cache.AC && CreateEmptyDBsOnFailure)
3841
Cache.AC = llvm::cas::createInMemoryActionCache();
3942
return {Cache.CAS, Cache.AC};
4043
}
4144

45+
llvm::Expected<std::pair<std::shared_ptr<llvm::cas::ObjectStore>,
46+
std::shared_ptr<llvm::cas::ActionCache>>>
47+
CASOptions::getOrCreateDatabases() const {
48+
if (auto E = initCache())
49+
return std::move(E);
50+
return std::pair{Cache.CAS, Cache.AC};
51+
}
52+
4253
void CASOptions::freezeConfig(DiagnosticsEngine &Diags) {
4354
if (Cache.Config.IsFrozen)
4455
return;
4556

4657
// Make sure the cache is initialized.
47-
initCache(Diags);
58+
if (auto E = initCache())
59+
Diags.Report(diag::err_cas_cannot_be_initialized) << toString(std::move(E));
4860

4961
// Freeze the CAS and wipe out the visible config to hide it from future
5062
// accesses. For example, future diagnostics cannot see this. Something that
@@ -77,10 +89,10 @@ void CASOptions::ensurePersistentCAS() {
7789
}
7890
}
7991

80-
void CASOptions::initCache(DiagnosticsEngine &Diags) const {
92+
llvm::Error CASOptions::initCache() const {
8193
auto &CurrentConfig = static_cast<const CASConfiguration &>(*this);
8294
if (CurrentConfig == Cache.Config && Cache.CAS && Cache.AC)
83-
return;
95+
return llvm::Error::success();
8496

8597
Cache.Config = CurrentConfig;
8698
StringRef CASPath = Cache.Config.CASPath;
@@ -90,18 +102,16 @@ void CASOptions::initCache(DiagnosticsEngine &Diags) const {
90102
if (llvm::Error E =
91103
createPluginCASDatabases(PluginPath, CASPath, PluginOptions)
92104
.moveInto(DBs)) {
93-
Diags.Report(diag::err_plugin_cas_cannot_be_initialized)
94-
<< PluginPath << toString(std::move(E));
95-
return;
105+
return E;
96106
}
97107
std::tie(Cache.CAS, Cache.AC) = std::move(DBs);
98-
return;
108+
return llvm::Error::success();
99109
}
100110

101111
if (CASPath.empty()) {
102112
Cache.CAS = llvm::cas::createInMemoryCAS();
103113
Cache.AC = llvm::cas::createInMemoryActionCache();
104-
return;
114+
return llvm::Error::success();
105115
}
106116

107117
SmallString<256> PathBuf;
@@ -110,10 +120,9 @@ void CASOptions::initCache(DiagnosticsEngine &Diags) const {
110120
CASPath = PathBuf;
111121
}
112122
std::pair<std::unique_ptr<ObjectStore>, std::unique_ptr<ActionCache>> DBs;
113-
if (llvm::Error E = createOnDiskUnifiedCASDatabases(CASPath).moveInto(DBs)) {
114-
Diags.Report(diag::err_builtin_cas_cannot_be_initialized)
115-
<< CASPath << toString(std::move(E));
116-
return;
117-
}
123+
if (llvm::Error E = createOnDiskUnifiedCASDatabases(CASPath).moveInto(DBs))
124+
return E;
125+
118126
std::tie(Cache.CAS, Cache.AC) = std::move(DBs);
127+
return llvm::Error::success();
119128
}

clang/test/CAS/plugin-cas.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@
4848
// RUN: -cc1 %s -fcas-path %t/cas \
4949
// RUN: -fcas-plugin-path %llvmshlibdir/libCASPluginTest%pluginext \
5050
// RUN: -fcas-plugin-option no-such-option=2 2>&1 | FileCheck %s --check-prefix=FAIL-PLUGIN-OPT
51-
// FAIL-PLUGIN-OPT: fatal error: plugin CAS cannot be initialized {{.*}}: unknown option: no-such-option
51+
// FAIL-PLUGIN-OPT: fatal error: CAS cannot be initialized from the specified '-fcas-*' options: unknown option: no-such-option
5252

5353
// RUN: not %clang -cc1depscan -o %t/t.rsp -fdepscan=inline -cc1-args \
5454
// RUN: -cc1 %s -fcas-path %t/cas -fcas-plugin-path %t/non-existent 2>&1 | FileCheck %s --check-prefix=NOTEXISTENT
55-
// NOTEXISTENT: fatal error: plugin CAS cannot be initialized
55+
// NOTEXISTENT: fatal error: CAS cannot be initialized from the specified '-fcas-*' options
5656

5757
#warning some warning
5858
void test() {}

clang/unittests/CAS/CASOptionsTest.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,30 @@ TEST(CASOptionsTest, freezeConfig) {
153153
EXPECT_EQ(CASOptions::UnknownCAS, Opts.getKind());
154154
}
155155

156+
TEST(CASOptionsTest, equal) {
157+
CASOptions Opt1, Opt2;
158+
ASSERT_TRUE(Opt1 == Opt2);
159+
160+
Opt1.CASPath = "some/path";
161+
Opt2.CASPath = "some/path";
162+
ASSERT_TRUE(Opt1 == Opt2);
163+
Opt2.CASPath = "other/path";
164+
ASSERT_TRUE(Opt1 != Opt2);
165+
166+
Opt1.CASPath.clear();
167+
Opt1.PluginPath = "plugin/path";
168+
ASSERT_TRUE(Opt1 != Opt2);
169+
Opt2.CASPath.clear();
170+
ASSERT_TRUE(Opt1 != Opt2);
171+
Opt2.PluginPath = "plugin/path2";
172+
ASSERT_TRUE(Opt1 != Opt2);
173+
Opt2.PluginPath = "plugin/path";
174+
ASSERT_TRUE(Opt1 == Opt2);
175+
176+
Opt1.PluginOptions.emplace_back("key", "value");
177+
ASSERT_TRUE(Opt1 != Opt2);
178+
Opt2.PluginOptions.emplace_back("key", "value");
179+
ASSERT_TRUE(Opt1 == Opt2);
180+
}
181+
156182
} // end namespace

0 commit comments

Comments
 (0)