[clang] Format "array is too large" diagnostics with separators #160260
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesWe only display this diagnostic with large numbers, so add the separators. Not sure if Full diff: https://github.com/llvm/llvm-project/pull/160260.diff 5 Files Affected:
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e10511cc7fc4e..9ef7a2698913d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6794,7 +6794,9 @@ bool Sema::tryToFixVariablyModifiedVarType(TypeSourceInfo *&TInfo,
if (SizeIsNegative)
Diag(Loc, diag::err_typecheck_negative_array_size);
else if (Oversized.getBoolValue())
- Diag(Loc, diag::err_array_too_large) << toString(Oversized, 10);
+ Diag(Loc, diag::err_array_too_large) << toString(
+ Oversized, 10, Oversized.isSigned(), /*formatAsCLiteral=*/false,
+ /*UpperCase=*/false, /*InsertSeparators=*/true);
else if (FailedFoldDiagID)
Diag(Loc, FailedFoldDiagID);
return false;
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 293097fd708fb..779ccf5f1e888 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2395,7 +2395,10 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context))
return ExprError(
Diag((*ArraySize)->getBeginLoc(), diag::err_array_too_large)
- << toString(*Value, 10) << (*ArraySize)->getSourceRange());
+ << toString(*Value, 10, Value->isSigned(),
+ /*formatAsCLiteral=*/false, /*UpperCase=*/false,
+ /*InsertSeparators=*/true)
+ << (*ArraySize)->getSourceRange());
}
KnownArraySize = Value->getZExtValue();
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index d723fb80f437e..bee613aa5f1c5 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -2270,7 +2270,10 @@ QualType Sema::BuildArrayType(QualType T, ArraySizeModifier ASM,
: ConstVal.getActiveBits();
if (ActiveSizeBits > ConstantArrayType::getMaxSizeBits(Context)) {
Diag(ArraySize->getBeginLoc(), diag::err_array_too_large)
- << toString(ConstVal, 10) << ArraySize->getSourceRange();
+ << toString(ConstVal, 10, ConstVal.isSigned(),
+ /*formatAsCLiteral=*/false, /*UpperCase=*/false,
+ /*InsertSeparators=*/true)
+ << ArraySize->getSourceRange();
return QualType();
}
diff --git a/clang/test/C/C23/n2838.c b/clang/test/C/C23/n2838.c
index cd20ea59884b2..c74f8cbe0a96c 100644
--- a/clang/test/C/C23/n2838.c
+++ b/clang/test/C/C23/n2838.c
@@ -4,9 +4,9 @@
* Types and sizes
*/
-char buffer4[0xFFFF'FFFF'FFFF'FFFF'1wb]; /* expected-error {{array is too large (295147905179352825841 elements)}} */
-char buffer3[0xFFFF'FFFF'FFFF'FFFFwb]; /* expected-error {{array is too large (18446744073709551615 elements)}} */
-char buffer2[0x7FFF'FFFF'FFFF'FFFFwb]; /* expected-error {{array is too large (9223372036854775807 elements)}} */
+char buffer4[0xFFFF'FFFF'FFFF'FFFF'1wb]; /* expected-error {{array is too large (295'147'905'179'352'825'841 elements)}} */
+char buffer3[0xFFFF'FFFF'FFFF'FFFFwb]; /* expected-error {{array is too large (18'446'744'073'709'551'615 elements)}} */
+char buffer2[0x7FFF'FFFF'FFFF'FFFFwb]; /* expected-error {{array is too large (9'223'372'036'854'775'807 elements)}} */
char buffer1[0x1FFF'FFFF'FFFF'FFFFwb]; /* array is juuuuuust right */
/* The largest object we can create is still smaller than SIZE_MAX. */
diff --git a/clang/test/C/drs/dr2xx.c b/clang/test/C/drs/dr2xx.c
index ffdf5aac377d9..7567d485d4324 100644
--- a/clang/test/C/drs/dr2xx.c
+++ b/clang/test/C/drs/dr2xx.c
@@ -370,7 +370,7 @@ void dr266(void) {
*/
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wlong-long"
- (void)sizeof(int[__SIZE_MAX__ / 2][__SIZE_MAX__ / 2]); /* expected-error-re 2 {{array is too large ({{[0-9]+}} elements)}} */
+ (void)sizeof(int[__SIZE_MAX__ / 2][__SIZE_MAX__ / 2]); /* expected-error-re 2 {{array is too large ({{[0-9']+}} elements)}} */
#pragma clang diagnostic pop
}
|
Sirraide
approved these changes
Sep 23, 2025
Yeah, it definitely helps I’d say; I wonder if we should insert separators by default for large numbers but that might not make sense in every context. |
|
It is unfortunately harder than it sounds since we never copy the AP(S)Int into the diagnostic right now :( |
I see, yeah. In that case just doing that for diagnostics that are likely to print large numbers is still nice. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/15197 Here is the relevant piece of the build log for the reference |
shafik
reviewed
Sep 24, 2025
| else if (Oversized.getBoolValue()) | ||
| Diag(Loc, diag::err_array_too_large) << toString(Oversized, 10); | ||
| Diag(Loc, diag::err_array_too_large) << toString( | ||
| Oversized, 10, Oversized.isSigned(), /*formatAsCLiteral=*/false, |
There was a problem hiding this comment.
10 -> /*Radix=*/10
Same below
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We only display this diagnostic with large numbers, so add the separators.
Not sure if
295'147'905'179'352'825'841is much better than295147905179352825841but I think it would be nice to have separators in more diagnostics.