Skip to content

Commit e3c2483

Browse files
committed
[-Wunsafe-buffer-usage] Preserve array element type spelling in std::array fixits
In other words - don't use the type that the source code gets expanded to in the AST as there's very likely a reason why it is represented via a typedef or macro in the sourcode.
1 parent da47129 commit e3c2483

File tree

2 files changed

+38
-4
lines changed

2 files changed

+38
-4
lines changed

clang/lib/Analysis/UnsafeBufferUsage.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include "clang/AST/RecursiveASTVisitor.h"
1313
#include "clang/AST/StmtVisitor.h"
1414
#include "clang/ASTMatchers/ASTMatchFinder.h"
15+
#include "clang/Basic/CharInfo.h"
16+
#include "clang/Basic/SourceLocation.h"
1517
#include "clang/Lex/Lexer.h"
1618
#include "clang/Lex/Preprocessor.h"
1719
#include "llvm/ADT/APSInt.h"
@@ -2492,8 +2494,25 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
24922494
return {};
24932495
}
24942496

2497+
const SourceLocation IdentifierLoc = getVarDeclIdentifierLoc(D);
2498+
2499+
// Get the spelling of the element type as written in the source file (including macros, etc.).
2500+
auto MaybeElemTypeTxt = getRangeText({D->getBeginLoc(), IdentifierLoc}, Ctx.getSourceManager(), Ctx.getLangOpts());
2501+
if (!MaybeElemTypeTxt)
2502+
return {};
2503+
std::string ElemTypeTxt = MaybeElemTypeTxt->str();
2504+
// Trim whitespace from the type spelling.
2505+
unsigned TrailingWhitespace = 0;
2506+
for (auto It = ElemTypeTxt.rbegin(); It < ElemTypeTxt.rend(); ++It) {
2507+
if (!isWhitespace(*It))
2508+
break;
2509+
++TrailingWhitespace;
2510+
}
2511+
if (TrailingWhitespace > 0)
2512+
ElemTypeTxt.erase(ElemTypeTxt.length() - TrailingWhitespace);
2513+
24952514
// Find the '[' token.
2496-
std::optional<Token> NextTok = Lexer::findNextToken(getVarDeclIdentifierLoc(D), Ctx.getSourceManager(), Ctx.getLangOpts());
2515+
std::optional<Token> NextTok = Lexer::findNextToken(IdentifierLoc, Ctx.getSourceManager(), Ctx.getLangOpts());
24972516
while (NextTok && !NextTok->is(tok::l_square))
24982517
NextTok = Lexer::findNextToken(NextTok->getLocation(), Ctx.getSourceManager(), Ctx.getLangOpts());
24992518
if (!NextTok)
@@ -2517,7 +2536,7 @@ static FixItList fixVarDeclWithArray(const VarDecl *D, const ASTContext &Ctx,
25172536

25182537
SmallString<32> Replacement;
25192538
raw_svector_ostream OS(Replacement);
2520-
OS << "std::array<" << ArrayEltT.getAsString() << ", "
2539+
OS << "std::array<" << ElemTypeTxt << ", "
25212540
<< ArraySizeTxt << "> " << IdentText->str();
25222541

25232542
FixIts.push_back(FixItHint::CreateReplacement(

clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-array.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ void weird_whitespace_in_declaration(unsigned idx) {
1818

1919
void weird_comments_in_declaration(unsigned idx) {
2020
int /* [ ] */ buffer_w /* [ ] */ [ /* [ ] */ 10 /* [ ] */ ] ;
21-
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int, /* [ ] */ 10 /* [ ] */ > buffer_w"
21+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:67}:"std::array<int /* [ ] */, /* [ ] */ 10 /* [ ] */ > buffer_w"
2222
buffer_w[idx] = 0;
2323
}
2424

@@ -65,6 +65,21 @@ void local_array_in_template(unsigned idx) {
6565
// Instantiate the template function to force its analysis.
6666
template void local_array_in_template<int>(unsigned); // FIXME: expected note {{in instantiation of}}
6767

68+
typedef unsigned int uint;
69+
void typedef_as_elem_type(unsigned idx) {
70+
uint buffer[10];
71+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:18}:"std::array<uint, 10> buffer"
72+
buffer[idx] = 0;
73+
}
74+
75+
void macro_as_elem_type(unsigned idx) {
76+
#define MY_INT int
77+
MY_INT buffer[10];
78+
// FIXME: implement support
79+
buffer[idx] = 0;
80+
#undef MY_INT
81+
}
82+
6883
void macro_as_identifier(unsigned idx) {
6984
#define MY_BUFFER buffer
7085
int MY_BUFFER[10];
@@ -76,7 +91,7 @@ void macro_as_identifier(unsigned idx) {
7691
void macro_as_size(unsigned idx) {
7792
#define MY_TEN 10
7893
int buffer[MY_TEN];
79-
// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]
94+
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:21}:"std::array<int, MY_TEN> buffer"
8095
buffer[idx] = 0;
8196
#undef MY_TEN
8297
}

0 commit comments

Comments
 (0)