llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (wheatman) <details> <summary>Changes</summary> This is to address https://github.com/llvm/llvm-project/issues/18763 it removes warnings from using a signed char as an array bound if the char is a known positives constant. This goes one step farther than gcc does. For example given the following code ```c++ char upper[300]; int main() { upper['a'] = 'A'; char b = 'a'; upper[b] = 'A'; const char c = 'a'; upper[c] = 'A'; constexpr char d = 'a'; upper[d] = 'A'; char e = -1; upper[e] = 'A'; const char f = -1; upper[f] = 'A'; constexpr char g = -1; upper[g] = 'A'; return 1; } ``` clang currently gives warnings for all cases, while gcc gives warnings for all cases except for 'a' (https://godbolt.org/z/5ahjETTv3) with the change there is no longer any warning for 'a', 'c', or 'd'. ``` ../test.cpp:7:8: warning: array subscript is of type 'char' [-Wchar-subscripts] 7 | upper[b] = 'A'; | ^~ ../test.cpp:13:8: warning: array subscript is of type 'char' [-Wchar-subscripts] 13 | upper[e] = 'A'; | ^~ ../test.cpp:15:8: warning: array subscript is of type 'char' [-Wchar-subscripts] 15 | upper[f] = 'A'; | ^~ ../test.cpp:17:8: warning: array subscript is of type 'char' [-Wchar-subscripts] 17 | upper[g] = 'A'; | ^~ ../test.cpp:15:3: warning: array index -1 is before the beginning of the array [-Warray-bounds] 15 | upper[f] = 'A'; | ^ ~ ../test.cpp:1:1: note: array 'upper' declared here 1 | char upper[300]; | ^ ../test.cpp:17:3: warning: array index -1 is before the beginning of the array [-Warray-bounds] 17 | upper[g] = 'A'; | ^ ~ ../test.cpp:1:1: note: array 'upper' declared here 1 | char upper[300]; | ^ 6 warnings generated. ``` This pull request in incomplete in that this is my first change submitted and I don't know how to add tests, any guidance on what sort of tests to add, and where to find documentation on the testing infrastructure --- Full diff: https://github.com/llvm/llvm-project/pull/69061.diff 1 Files Affected: - (modified) clang/lib/Sema/SemaExpr.cpp (+9-3) ``````````diff diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index aa30a3a03887558..dd9ba5cecaf2404 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -6018,9 +6018,15 @@ Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc, << IndexExpr->getSourceRange()); if ((IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_S) || - IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_U)) - && !IndexExpr->isTypeDependent()) - Diag(LLoc, diag::warn_subscript_is_char) << IndexExpr->getSourceRange(); + IndexExpr->getType()->isSpecificBuiltinType(BuiltinType::Char_U)) && + !IndexExpr->isTypeDependent()) { + std::optional<llvm::APSInt> IntegerContantExpr = + IndexExpr->getIntegerConstantExpr(getASTContext()); + if (!(IntegerContantExpr.has_value() && + IntegerContantExpr.value().isNonNegative())) { + Diag(LLoc, diag::warn_subscript_is_char) << IndexExpr->getSourceRange(); + } + } // C99 6.5.2.1p1: "shall have type "pointer to *object* type". Similarly, // C++ [expr.sub]p1: The type "T" shall be a completely-defined object `````````` </details> https://github.com/llvm/llvm-project/pull/69061 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
