Skip to content

Commit 87b847d

Browse files
committed
[analysis] FunctionAnalysisBase's parameter SILAnalysisTy is actually a FunctionInfoTy.
I believe that this was just a typo from a long time ago. Calling this parameter a SILAnalysisTy is actively misleading since as a result it seems to a naive reading that one should be writing a recursive template: ``` class MyAnalysis : public FunctionAnalysisBase<MyAnalysis> { ... } ``` Instead of passing in the function info of the derived analysis, i.e.: ``` class MyAnalysisFunctionInfo { ... } class MyAnalysis : public FunctionAnalysisBase<MyAnalysisFunctionInfo> { ... } ``` I also added some documentation to that affect onto FunctionAnalysisBase.
1 parent 7200080 commit 87b847d

File tree

1 file changed

+33
-17
lines changed

1 file changed

+33
-17
lines changed

include/swift/SILOptimizer/Analysis/Analysis.h

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -157,24 +157,39 @@ class AnalysisPreserver {
157157

158158
/// An abstract base class that implements the boiler plate of caching and
159159
/// invalidating analysis for specific functions.
160-
template<typename AnalysisTy>
160+
///
161+
/// The usage expectation is that the derived function analysis will inherit
162+
/// from FunctionAnalysisBase and pass in as a template argument the
163+
/// "FunctionInfoTy" struct as a template argument. The FunctionInfoTy struct
164+
/// should represent all of the information that the analysis should store about
165+
/// an individual function. As a toy example:
166+
///
167+
/// ```
168+
/// struct TriviallyDeadAnalysisFunctionInfo {
169+
/// bool isTriviallyDead;
170+
/// };
171+
///
172+
/// class TriviallyDeadAnalysis
173+
/// : public FunctionAnalysisBase<TriviallyDeadAnalysisFunctionInfo> { ... }
174+
/// ```
175+
template <typename FunctionInfoTy>
161176
class FunctionAnalysisBase : public SILAnalysis {
162177
protected:
163-
using StorageTy = llvm::DenseMap<SILFunction *, AnalysisTy *>;
178+
using StorageTy = llvm::DenseMap<SILFunction *, FunctionInfoTy *>;
164179

165180
/// Maps functions to their analysis provider.
166181
StorageTy storage;
167182

168-
/// Construct a new empty analysis for a specific function \p F.
169-
virtual AnalysisTy *newFunctionAnalysis(SILFunction *f) = 0;
183+
/// Construct a new empty function info for a specific function \p F.
184+
virtual FunctionInfoTy *newFunctionAnalysis(SILFunction *f) = 0;
170185

171186
/// Return True if the analysis should be invalidated given trait \K is
172187
/// preserved.
173188
virtual bool shouldInvalidate(SILAnalysis::InvalidationKind k) = 0;
174189

175190
/// A stub function that verifies the specific AnalysisTy \p A. This is
176191
/// meant to be overridden by subclasses.
177-
virtual void verify(AnalysisTy *A) const {}
192+
virtual void verify(FunctionInfoTy *funcInfo) const {}
178193

179194
void deleteAllAnalysisProviders() {
180195
for (auto iter : storage)
@@ -183,21 +198,21 @@ class FunctionAnalysisBase : public SILAnalysis {
183198
}
184199

185200
public:
186-
/// Returns true if we have an analysis for a specific function \p F without
187-
/// actually constructing it.
188-
bool hasAnalysis(SILFunction *f) const { return storage.count(f); }
201+
/// Returns true if we have data for a specific function \p F without actually
202+
/// attempting to construct the function info.
203+
bool hasFunctionInfo(SILFunction *f) const { return storage.count(f); }
189204

190205
/// Attempt to lookup up the information that the analysis has for the given
191206
/// function. Returns nullptr upon failure.
192-
NullablePtr<AnalysisTy> maybeGet(SILFunction *f) {
207+
NullablePtr<FunctionInfoTy> maybeGet(SILFunction *f) {
193208
auto iter = storage.find(f);
194209
if (iter == storage.end())
195210
return nullptr;
196211
return iter->second;
197212
}
198213

199-
/// Returns an analysis provider for a specific function \p F.
200-
AnalysisTy *get(SILFunction *f) {
214+
/// Returns a function info structure for a specific function \p F.
215+
FunctionInfoTy *get(SILFunction *f) {
201216
// Check that the analysis can handle this function.
202217
verifyFunction(f);
203218

@@ -212,7 +227,7 @@ class FunctionAnalysisBase : public SILAnalysis {
212227
deleteAllAnalysisProviders();
213228
}
214229

215-
/// Helper function to remove the analysis data for a function.
230+
/// Helper function to remove the function info for a specific function.
216231
void invalidateFunction(SILFunction *f) {
217232
auto &it = storage.FindAndConstruct(f);
218233
if (!it.second)
@@ -244,14 +259,15 @@ class FunctionAnalysisBase : public SILAnalysis {
244259
virtual ~FunctionAnalysisBase() {
245260
deleteAllAnalysisProviders();
246261
}
262+
247263
FunctionAnalysisBase(SILAnalysisKind k) : SILAnalysis(k), storage() {}
248264
FunctionAnalysisBase(const FunctionAnalysisBase &) = delete;
249265
FunctionAnalysisBase &operator=(const FunctionAnalysisBase &) = delete;
250266

251267
/// Verify all of the AnalysisTy for all functions.
252268
///
253-
/// This is not meant to be overridden by subclasses. See "void
254-
/// verify(AnalysisTy *A)".
269+
/// This is not meant to be overridden by subclasses. Instead please override
270+
/// void FunctionAnalysisBase::verify(FunctionInfoTy *fInfo).
255271
virtual void verify() const override final {
256272
for (auto iter : storage) {
257273
if (!iter.second)
@@ -260,11 +276,11 @@ class FunctionAnalysisBase : public SILAnalysis {
260276
}
261277
}
262278

263-
/// Verify the AnalysisTy that we have stored for the specific function \p
279+
/// Verify the FunctionInfoTy that we have stored for the specific function \p
264280
/// F.
265281
///
266-
/// This is not meant to be overridden by subclasses. See "void
267-
/// verify(AnalysisTy *analysis)".
282+
/// This is not meant to be overridden by subclasses. Instead, please
283+
/// override: void FunctionAnalysisBase::verify(FunctionInfoTy *fInfo).
268284
virtual void verify(SILFunction *f) const override final {
269285
auto iter = storage.find(f);
270286
if (iter == storage.end())

0 commit comments

Comments
 (0)