Skip to content

Conversation

@steakhal
Copy link
Contributor

No description provided.

@steakhal steakhal added the clang Clang issues not falling into any other category label Jan 31, 2025
@steakhal steakhal changed the title [clang][NFC] Simplify code in ExprMutationAnalyzer [clang][mutation analyzer][NFC] Simplify code in ExprMutationAnalyzer Jan 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Author: Balazs Benics (steakhal)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/125283.diff

1 Files Affected:

  • (modified) clang/lib/Analysis/ExprMutationAnalyzer.cpp (+1-4)
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index d7b44149d0fc4b..93376eb9b93c72 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -813,10 +813,7 @@ FunctionParmMutationAnalyzer::findMutation(const ParmVarDecl *Parm) {
   // before analyzing parameters of A. Then when analyzing the second "call A",
   // FunctionParmMutationAnalyzer can use this memoized value to avoid infinite
   // recursion.
-  Results[Parm] = nullptr;
-  if (const Stmt *S = BodyAnalyzer.findMutation(Parm))
-    return Results[Parm] = S;
-  return Results[Parm];
+  return Results[Parm] = BodyAnalyzer.findMutation(Parm);
 }
 
 } // namespace clang

if (const Stmt *S = BodyAnalyzer.findMutation(Parm))
return Results[Parm] = S;
return Results[Parm];
return Results[Parm] = BodyAnalyzer.findMutation(Parm);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have the following a few lines above:

  const auto Memoized = Results.find(Parm);
  if (Memoized != Results.end())
    return Memoized->second;

Could we turn that into something like this and use the iterator from try_emplace?

  auto [Memoized, Inserted] = Results.try_emplace(Parm);
  if (!Inserted)
    return Memoized->second;

  Memoized->second = ...;

Maybe we should rename Memorized. The name would sound strange if the insertion is successful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I usually use "Place" or "Slot" for this.

Copy link
Contributor

@kazutakahirata kazutakahirata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you for the update!

@steakhal steakhal merged commit 0d21ef4 into llvm:main Feb 1, 2025
8 checks passed
@steakhal steakhal deleted the bb/nfc-expr-mutation-remove-double-lookup branch February 1, 2025 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:analysis clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants