[clang] 726d7b0 - [Sema] Simplify CheckConstraintSatisfaction. NFC

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2022-05-04T15:53:07Z
New Revision: 726d7b07fcde58c61743c45723c9b614911fe084

URL: https://github.com/llvm/llvm-project/commit/726d7b07fcde58c61743c45723c9b614911fe084
DIFF: https://github.com/llvm/llvm-project/commit/726d7b07fcde58c61743c45723c9b614911fe084.diff

LOG: [Sema] Simplify CheckConstraintSatisfaction. NFC

- Exit early when constraint caching is disabled.
- Use unique_ptr to manage temporary lifetime.
- Fix a typo in a comment (InsertPos instead of InsertNode).

The new code duplicates the forwarding call to CheckConstraintSatisfaction,
but reduces the number of interconnected if statements and simplifies lifetime
management.

This increases the overall readability.

Reviewed By: sammccall

Differential Revision: https://reviews.llvm.org/D124923

Added: 
    

Modified: 
    clang/lib/Sema/SemaConcept.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 449f9eb33f47b..4ec37a94089b0 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,31 @@ bool Sema::CheckConstraintSatisfaction(
     return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+    return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+                                         TemplateArgs, TemplateIDRange,
+                                         OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-    ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-    Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-    if (Satisfaction) {
-      OutSatisfaction = *Satisfaction;
-      return false;
-    }
-    Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-    Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+    OutSatisfaction = *Cached;
+    return false;
   }
+  auto Satisfaction =
+      std::make_unique<ConstraintSatisfaction>(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
                                     TemplateArgs, TemplateIDRange,
                                     *Satisfaction)) {
-    if (ShouldCache)
-      delete Satisfaction;
     return true;
   }
-
-  if (ShouldCache) {
-    // We cannot use InsertNode here because CheckConstraintSatisfaction might
-    // have invalidated it.
-    SatisfactionCache.InsertNode(Satisfaction);
-    OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might have
+  // invalidated it.
+  // FIXME: this leaks memory, we should allocate in the arena instead.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }