[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Ben Shi via cfe-commits]

https://github.com/benshi001 updated 
https://github.com/llvm/llvm-project/pull/71394

>From 3be57e39926bda29b273d5e9e01bff5ec8b2302e Mon Sep 17 00:00:00 2001
From: Ben Shi 
Date: Tue, 7 Nov 2023 14:11:37 +0800
Subject: [PATCH] [clang][analyzer][NFC] Remove redundant code in StreamChecker

---
 clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 898906977ba9bb6..69610029beb0a9e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -1093,7 +1093,6 @@ ProgramStateRef StreamChecker::ensureStreamOpened(SVal 
StreamVal,
   N));
   return nullptr;
 }
-return State;
   }
 
   return State;

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

balazske wrote:

With the current code it is a corner case if this change makes the code more 
readable. Probably it can be useful if new functions are added to the checker. 
But the rule here is that there is one "ensure" function to check one aspect of 
the state, and the pre-callbacks call all of the ensure functions that are 
needed in that case. It would not much more readable if multiple combinations 
of ensure functions are made, for example 
`ensureStreamNonNullAndOpenedAndNoFilePositionIndeterminate`.

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

@@ -639,12 +644,7 @@ void StreamChecker::preFreadFwrite(const FnDescription 
*Desc,
bool IsFread) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))

balazske wrote:

The function returns a state that is modified further by the following 
functions. Otherwise the state changes applied in `basicCheck` are lost.  
(`ensureStreamNonNull` does a state change, `ensureStreamOpened` does not, but 
all return a state to make the usage similar.)

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

https://github.com/balazske edited 
https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

@@ -342,6 +342,11 @@ class StreamChecker : public Checkerhttps://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

@@ -639,12 +644,7 @@ void StreamChecker::preFreadFwrite(const FnDescription 
*Desc,
bool IsFread) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))

balazske wrote:

```suggestion
  State = ensureStreamNonNullAndOpened(Desc, Call, C, State, StreamVal);
  if (!State)
return;
```

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balázs Kéri via cfe-commits]

https://github.com/balazske requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Balazs Benics via cfe-commits]

https://github.com/steakhal requested changes to this pull request.

I find this alternative less idiomatic (dissimilar to how we implement checks 
in other checkers), thus I find this less readable.
I'm okay with that amount of redundancy as it was present.

https://github.com/llvm/llvm-project/pull/71394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Ben Shi via cfe-commits]

https://github.com/benshi001 updated 
https://github.com/llvm/llvm-project/pull/71394

>From 56d5604cf0442919d62def08c233e8d48b654885 Mon Sep 17 00:00:00 2001
From: Ben Shi 
Date: Mon, 6 Nov 2023 21:49:36 +0800
Subject: [PATCH] [clang][Analyzer][NFC] Simplify
 preDefault/preFseek/preFreadFwrite of StreamChecker

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 33 +--
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 898906977ba9bb6..877e5d2a09f398c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -342,6 +342,11 @@ class StreamChecker : public CheckerStreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))
 return;
   State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
   if (!State)
@@ -749,12 +749,7 @@ void StreamChecker::preFseek(const FnDescription *Desc, 
const CallEvent ,
  CheckerContext ) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))
 return;
   State = ensureFseekWhenceCorrect(Call.getArgSVal(2), C, State);
   if (!State)
@@ -1002,15 +997,19 @@ void StreamChecker::preDefault(const FnDescription 
*Desc, const CallEvent ,
CheckerContext ) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
+  if (basicCheck(Desc, Call, C, State, StreamVal))
+C.addTransition(State);
+}
+
+bool StreamChecker::basicCheck(const FnDescription *Desc, const CallEvent 
,
+   CheckerContext , ProgramStateRef ,
+   SVal ) const {
   State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
   State);
   if (!State)
-return;
+return false;
   State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
-return;
-
-  C.addTransition(State);
+  return State != nullptr;
 }
 
 void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Ben Shi via cfe-commits]

https://github.com/benshi001 updated 
https://github.com/llvm/llvm-project/pull/71394

>From abbca31776cf4223392726d64aadfa5c79b57a69 Mon Sep 17 00:00:00 2001
From: Ben Shi 
Date: Mon, 6 Nov 2023 21:49:36 +0800
Subject: [PATCH] [clang][Analyzer][NFC] Simplify
 preDefault/preFseek/preFreadFwrite of StreamChecker

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 34 +--
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 898906977ba9bb6..c5c33979b202154 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -342,6 +342,11 @@ class StreamChecker : public CheckerStreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))
 return;
+
   State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
   if (!State)
 return;
@@ -749,12 +750,7 @@ void StreamChecker::preFseek(const FnDescription *Desc, 
const CallEvent ,
  CheckerContext ) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))
 return;
   State = ensureFseekWhenceCorrect(Call.getArgSVal(2), C, State);
   if (!State)
@@ -1002,15 +998,19 @@ void StreamChecker::preDefault(const FnDescription 
*Desc, const CallEvent ,
CheckerContext ) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
+  if (basicCheck(Desc, Call, C, State, StreamVal))
+C.addTransition(State);
+}
+
+bool StreamChecker::basicCheck(const FnDescription *Desc, const CallEvent 
,
+   CheckerContext , ProgramStateRef ,
+   SVal ) const {
   State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
   State);
   if (!State)
-return;
+return false;
   State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
-return;
-
-  C.addTransition(State);
+  return State != nullptr;
 }
 
 void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][Analyzer][NFC] Simplify preDefault/preFseek/preFreadFwrite of StreamChecker (PR #71394)

[Ben Shi via cfe-commits]

https://github.com/benshi001 created 
https://github.com/llvm/llvm-project/pull/71394

None

>From 965c109cc19187329d5ab2ae324665dfbd7c17ee Mon Sep 17 00:00:00 2001
From: Ben Shi 
Date: Mon, 6 Nov 2023 21:49:36 +0800
Subject: [PATCH] [clang][Analyzer][NFC] Simplify
 preDefault/preFseek/preFreadFwrite of StreamChecker

---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index 898906977ba9bb6..e35929c4dcd28e4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -342,6 +342,11 @@ class StreamChecker : public CheckerStreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))
 return;
+
   State = ensureNoFilePositionIndeterminate(StreamVal, C, State);
   if (!State)
 return;
@@ -749,13 +750,9 @@ void StreamChecker::preFseek(const FnDescription *Desc, 
const CallEvent ,
  CheckerContext ) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
-  State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
-  State);
-  if (!State)
-return;
-  State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
+  if (!basicCheck(Desc, Call, C, State, StreamVal))
 return;
+
   State = ensureFseekWhenceCorrect(Call.getArgSVal(2), C, State);
   if (!State)
 return;
@@ -1002,15 +999,19 @@ void StreamChecker::preDefault(const FnDescription 
*Desc, const CallEvent ,
CheckerContext ) const {
   ProgramStateRef State = C.getState();
   SVal StreamVal = getStreamArg(Desc, Call);
+  if (basicCheck(Desc, Call, C, State, StreamVal))
+C.addTransition(State);
+}
+
+bool StreamChecker::basicCheck(const FnDescription *Desc, const CallEvent 
,
+   CheckerContext , ProgramStateRef ,
+   SVal ) const {
   State = ensureStreamNonNull(StreamVal, Call.getArgExpr(Desc->StreamArgNo), C,
   State);
   if (!State)
-return;
+return false;
   State = ensureStreamOpened(StreamVal, C, State);
-  if (!State)
-return;
-
-  C.addTransition(State);
+  return State != nullptr;
 }
 
 void StreamChecker::evalSetFeofFerror(const FnDescription *Desc,

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits