riccibruno created this revision.
riccibruno added reviewers: aaron.ballman, rsmith.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
Do the following NFCs, before doing anything more substantial:
1. Add some comments to hopefully reduce the amount of code staring for the
next guy.
2. Pack `UsageInfo` by storing the using expressions and sequencing regions
separately. There is no point in wasting space here.
Repository:
rC Clang
https://reviews.llvm.org/D57659
Files:
lib/Sema/SemaChecking.cpp
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11733,20 +11733,38 @@
UK_Count = UK_ModAsSideEffect + 1
};
+ /// Bundle together a sequencing region and the expression corresponding
+ /// to a specific usage. One Usage is stored for each usage kind in UsageInfo.
struct Usage {
- Expr *Use;
+ Expr *UsageExpr = nullptr;
SequenceTree::Seq Seq;
-
- Usage() : Use(nullptr), Seq() {}
+ Usage() = default;
+ Usage(Expr *UsageExpr, SequenceTree::Seq Seq)
+ : UsageExpr(UsageExpr), Seq(Seq) {}
};
- struct UsageInfo {
- Usage Uses[UK_Count];
+ class UsageInfo {
+ /// The expressions corresponding to each usage kind.
+ Expr *UsageExprs[UK_Count]{};
+
+ /// The sequencing regions corresponding to each usage kind.
+ SequenceTree::Seq Seqs[UK_Count]{};
- /// Have we issued a diagnostic for this variable already?
- bool Diagnosed;
+ /// Have we issued a diagnostic for this object already?
+ bool Diagnosed = false;
- UsageInfo() : Uses(), Diagnosed(false) {}
+ public:
+ Usage getUsage(UsageKind UK) const {
+ assert(UK < UK_Count && "Invalid UsageKind!");
+ return Usage{UsageExprs[UK], Seqs[UK]};
+ }
+ void setUsage(UsageKind UK, Usage U) {
+ assert(UK < UK_Count && "Invalid UsageKind!");
+ UsageExprs[UK] = U.UsageExpr;
+ Seqs[UK] = U.Seq;
+ }
+ void markDiagnosed() { Diagnosed = true; }
+ bool alreadyDiagnosed() const { return Diagnosed; }
};
using UsageInfoMap = llvm::SmallDenseMap<Object, UsageInfo, 16>;
@@ -11781,11 +11799,14 @@
}
~SequencedSubexpression() {
- for (auto &M : llvm::reverse(ModAsSideEffect)) {
- UsageInfo &U = Self.UsageMap[M.first];
- auto &SideEffectUsage = U.Uses[UK_ModAsSideEffect];
- Self.addUsage(U, M.first, SideEffectUsage.Use, UK_ModAsValue);
- SideEffectUsage = M.second;
+ for (std::pair<Object, Usage> &M : llvm::reverse(ModAsSideEffect)) {
+ // Add a new usage with usage kind UK_ModAsValue, and then restore
+ // the previous usage with UK_ModAsSideEffect (thus clearing it if
+ // the previous one was empty).
+ UsageInfo &UI = Self.UsageMap[M.first];
+ Usage SideEffectUsage = UI.getUsage(UK_ModAsSideEffect);
+ Self.addUsage(M.first, UI, SideEffectUsage.UsageExpr, UK_ModAsValue);
+ UI.setUsage(UK_ModAsSideEffect, M.second);
}
Self.ModAsSideEffect = OldModAsSideEffect;
}
@@ -11848,28 +11869,34 @@
}
/// Note that an object was modified or used by an expression.
- void addUsage(UsageInfo &UI, Object O, Expr *Ref, UsageKind UK) {
- Usage &U = UI.Uses[UK];
- if (!U.Use || !Tree.isUnsequenced(Region, U.Seq)) {
+ /// UI is the UsageInfo for the object O as obtained via the UsageMap.
+ void addUsage(Object O, UsageInfo &UI, Expr *UsageExpr, UsageKind UK) {
+ // Get the old usage for the given object and usage kind.
+ Usage U = UI.getUsage(UK);
+ if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq)) {
+ // If we have a modification as side effect and are in a sequenced
+ // subexpression, save the old Usage so that we can restore it later
+ // in SequencedSubexpression::~SequencedSubexpression.
if (UK == UK_ModAsSideEffect && ModAsSideEffect)
ModAsSideEffect->push_back(std::make_pair(O, U));
- U.Use = Ref;
- U.Seq = Region;
+ // Then record the new usage with the current sequencing region.
+ UI.setUsage(UK, Usage(UsageExpr, Region));
}
}
/// Check whether a modification or use conflicts with a prior usage.
- void checkUsage(Object O, UsageInfo &UI, Expr *Ref, UsageKind OtherKind,
+ /// UI is the UsageInfo for the object O as obtained via the UsageMap.
+ void checkUsage(Object O, UsageInfo &UI, Expr *UsageExpr, UsageKind OtherKind,
bool IsModMod) {
- if (UI.Diagnosed)
+ if (UI.alreadyDiagnosed())
return;
- const Usage &U = UI.Uses[OtherKind];
- if (!U.Use || !Tree.isUnsequenced(Region, U.Seq))
+ Usage U = UI.getUsage(OtherKind);
+ if (!U.UsageExpr || !Tree.isUnsequenced(Region, U.Seq))
return;
- Expr *Mod = U.Use;
- Expr *ModOrUse = Ref;
+ Expr *Mod = U.UsageExpr;
+ Expr *ModOrUse = UsageExpr;
if (OtherKind == UK_Use)
std::swap(Mod, ModOrUse);
@@ -11877,32 +11904,60 @@
IsModMod ? diag::warn_unsequenced_mod_mod
: diag::warn_unsequenced_mod_use)
<< O << SourceRange(ModOrUse->getExprLoc());
- UI.Diagnosed = true;
+ UI.markDiagnosed();
}
- void notePreUse(Object O, Expr *Use) {
- UsageInfo &U = UsageMap[O];
+ // A note on note{Pre, Post}{Use, Mod}:
+ //
+ // (It helps to follow the algorithm with an expression such as
+ // "((++k)++, k) = k" or "k = (k++, k++)". Both contain unsequenced
+ // operations before C++17 and both are well-defined in C++17).
+ //
+ // When visiting a node which uses/modify an object we first call notePreUse
+ // or notePreMod before visiting its sub-expression(s). At this point the
+ // children of the current node have not yet been visited and so the eventual
+ // uses/modifications resulting from the children of the current node have not
+ // been recorded yet.
+ //
+ // We then visit the children of the current node. After that notePostUse or
+ // notePostMod is called. These will 1) detect an unsequenced modification
+ // as side effect (as in "k++ + k") and 2) add a new usage with the
+ // appropriate usage kind.
+ //
+ // A last thing we have to be careful is that some operation sequences
+ // modification as side effect as well (for example: || or ,). To account for
+ // this we wrap the visitation of such a sub-expression (for example: the LHS
+ // of || or ,) with SequencedSubexpression. SequencedSubexpression is an RAII
+ // object which record usages which are modifications as side effect, and then
+ // downgrade them (or more accurately restore the previous usage which was a
+ // modification as side effect) when exiting the scope of the sequenced
+ // subexpression.
+
+ void notePreUse(Object O, Expr *UseExpr) {
+ UsageInfo &UI = UsageMap[O];
// Uses conflict with other modifications.
- checkUsage(O, U, Use, UK_ModAsValue, false);
+ checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsValue, /*IsModMod=*/false);
}
- void notePostUse(Object O, Expr *Use) {
- UsageInfo &U = UsageMap[O];
- checkUsage(O, U, Use, UK_ModAsSideEffect, false);
- addUsage(U, O, Use, UK_Use);
+ void notePostUse(Object O, Expr *UseExpr) {
+ UsageInfo &UI = UsageMap[O];
+ checkUsage(O, UI, UseExpr, /*OtherKind=*/UK_ModAsSideEffect,
+ /*IsModMod=*/false);
+ addUsage(O, UI, UseExpr, /*UsageKind=*/UK_Use);
}
- void notePreMod(Object O, Expr *Mod) {
- UsageInfo &U = UsageMap[O];
+ void notePreMod(Object O, Expr *ModExpr) {
+ UsageInfo &UI = UsageMap[O];
// Modifications conflict with other modifications and with uses.
- checkUsage(O, U, Mod, UK_ModAsValue, true);
- checkUsage(O, U, Mod, UK_Use, false);
+ checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsValue, /*IsModMod=*/true);
+ checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_Use, /*IsModMod=*/false);
}
- void notePostMod(Object O, Expr *Use, UsageKind UK) {
- UsageInfo &U = UsageMap[O];
- checkUsage(O, U, Use, UK_ModAsSideEffect, true);
- addUsage(U, O, Use, UK);
+ void notePostMod(Object O, Expr *ModExpr, UsageKind UK) {
+ UsageInfo &UI = UsageMap[O];
+ checkUsage(O, UI, ModExpr, /*OtherKind=*/UK_ModAsSideEffect,
+ /*IsModMod=*/true);
+ addUsage(O, UI, ModExpr, /*UsageKind=*/UK);
}
public:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
