================
@@ -0,0 +1,211 @@
+//===- BoundsChecking.h - Bounds checking related APIs ----------*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines APIs for performing a bounds check (i.e. comparing a
+//  symbolic Offset value to zero and a symbolic Extent value) and composing
+//  descriptions that explain its results.
+//
+//  This is intended as a replacement for `ProgramState::assumeInBound` to
+//  avoid its incorrect logic and compensate for deficiencies of other parts of
+//  the analyzer.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
+#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BOUNDSCHECKING_H
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <optional>
+
+namespace clang {
+namespace ento {
+
+/// If `E` is an array subscript expression with a base that is "clean" (= not
+/// modified by pointer arithmetic = the beginning of a memory region), return
+/// it as a pointer to ArraySubscriptExpr; otherwise return nullptr.
+/// This helper function is used by two separate heuristics that are only valid
+/// in these "clean" cases.
+const ArraySubscriptExpr *getAsCleanArraySubscriptExpr(const Expr *E,
----------------
NagyDonat wrote:

> Until it is moved, it would be great if this was not at the very beginning of 
> the header. The main readers of this file are the users of these APIs. Let's 
> try to not waste their time with info that they are not interested in. And if 
> we absolutely need to have some implementation details here, let's try to 
> move that towards the end.

If I move this to the end of the header file, I would need to move the 
definition of the trivial method `forExpr` (which uses it) to the `.cpp` file, 
which I don't like.

Also, this function will go away very soon (my next PR after this one will 
either delete it or at least move to `ArrayBoundChecker.cpp`) so please don't 
spend more time on it.

---------

> I also wonder if we can come up with a better term than `Clean`. Could be 
> like, `getZeroBasedArraySubscriptExpr` or 
> `getArraySubscriptExprFromBeginning`.

As this PR moves this function to a different file, I would strongly prefer to 
preserve its old name to provide continuity. I will probably eliminate this 
function in the non-NFC follow-up PR  that eliminates `SizeUnit::forExpr` (one 
of the two call sites; I will inline this at the other call site), which will 
make its name irrelevant.

By the way, for me the two suggested names are both confusing: as I see 
`ZeroBased` I'm immediately sidetracked by thoughts like "this is C/C++, all 
array indices are zero-based, why is this highlighted?", while  
`getArraySubscriptExprFromBeginning` is IMO as unclear as the current name.

https://github.com/llvm/llvm-project/pull/202372
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to