Skip to content

[clang][mutation analyzer][NFC] Simplify code in ExprMutationAnalyzer #125283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 1, 2025
Merged

[clang][mutation analyzer][NFC] Simplify code in ExprMutationAnalyzer #125283

merged 2 commits into from
Feb 1, 2025

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