[C2y] Handle FP-suffixes on prefixed octals (#141230) #141695
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Naveen Seth Hanig (naveen-seth) ChangesFixes #141230. Prefixed octal literals crash in combination with floating-point suffixes instead of rejecting them. No release note because this is fixing an issue with a new change. Full diff: https://github.com/llvm/llvm-project/pull/141695.diff 2 Files Affected:
diff --git a/clang/lib/Lex/LiteralSupport.cpp b/clang/lib/Lex/LiteralSupport.cpp
index 75ad977d64b24..3b941fc4c48b8 100644
--- a/clang/lib/Lex/LiteralSupport.cpp
+++ b/clang/lib/Lex/LiteralSupport.cpp
@@ -1420,7 +1420,7 @@ void NumericLiteralParser::ParseNumberStartingWithZero(SourceLocation TokLoc) {
}
// Parse a potential octal literal prefix.
- bool SawOctalPrefix = false, IsSingleZero = false;
+ bool IsSingleZero = false;
if ((c1 == 'O' || c1 == 'o') && (s[1] >= '0' && s[1] <= '7')) {
unsigned DiagId;
if (LangOpts.C2y)
@@ -1432,14 +1432,26 @@ void NumericLiteralParser::ParseNumberStartingWithZero(SourceLocation TokLoc) {
Diags.Report(TokLoc, DiagId);
++s;
DigitsBegin = s;
- SawOctalPrefix = true;
+ radix = 8;
+ s = SkipOctalDigits(s);
+ if (s == ThisTokEnd) {
+ // Done
+ } else if ((isHexDigit(*s) && *s != 'e' && *s != 'E' && *s != '.') &&
+ !isValidUDSuffix(LangOpts, StringRef(s, ThisTokEnd - s))) {
+ Diags.Report(Lexer::AdvanceToTokenCharacter(TokLoc, s - ThisTokBegin, SM,
+ LangOpts),
+ diag::err_invalid_digit)
+ << StringRef(s, 1) << 1;
+ hadError = true;
+ }
+ // Other suffixes will be diagnosed by the caller.
+ return;
}
auto _ = llvm::make_scope_exit([&] {
// If we still have an octal value but we did not see an octal prefix,
// diagnose as being an obsolescent feature starting in C2y.
- if (radix == 8 && LangOpts.C2y && !SawOctalPrefix && !hadError &&
- !IsSingleZero)
+ if (radix == 8 && LangOpts.C2y && !hadError && !IsSingleZero)
Diags.Report(TokLoc, diag::warn_unprefixed_octal_deprecated);
});
diff --git a/clang/test/C/C2y/n3353.c b/clang/test/C/C2y/n3353.c
index a616228f1bad0..d7d8b03501260 100644
--- a/clang/test/C/C2y/n3353.c
+++ b/clang/test/C/C2y/n3353.c
@@ -92,6 +92,28 @@ int o2 = 0xG; /* expected-error {{invalid suffix 'xG' on integer constant}}
c2y-warning {{octal literals without a '0o' prefix are deprecated}}
*/
+// Show that floating-point suffixes on octal literals are rejected.
+auto f1 = 0o0.; /* expected-error {{invalid suffix '.' on integer constant}}
+ compat-warning {{octal integer literals are incompatible with standards before C2y}}
+ ext-warning {{octal integer literals are a C2y extension}}
+ cpp-warning {{octal integer literals are a Clang extension}}
+ */
+auto f2 = 0o0.1; /* expected-error {{invalid suffix '.1' on integer constant}}
+ compat-warning {{octal integer literals are incompatible with standards before C2y}}
+ ext-warning {{octal integer literals are a C2y extension}}
+ cpp-warning {{octal integer literals are a Clang extension}}
+ */
+auto f3 = 0o0e1; /* expected-error {{invalid suffix 'e1' on integer constant}}
+ compat-warning {{octal integer literals are incompatible with standards before C2y}}
+ ext-warning {{octal integer literals are a C2y extension}}
+ cpp-warning {{octal integer literals are a Clang extension}}
+ */
+auto f4 = 0o0E1; /* expected-error {{invalid suffix 'E1' on integer constant}}
+ compat-warning {{octal integer literals are incompatible with standards before C2y}}
+ ext-warning {{octal integer literals are a C2y extension}}
+ cpp-warning {{octal integer literals are a Clang extension}}
+ */
+
// Ensure digit separators work as expected.
constexpr int p = 0o0'1'2'3'4'5'6'7; /* compat-warning {{octal integer literals are incompatible with standards before C2y}}
ext-warning {{octal integer literals are a C2y extension}}
|
naveen-seth
force-pushed
the
fix-n3353-float-suffix
branch
from
fd40edb to
9773316
Compare
|
Thank you for reviewing this so quickly! I was just about to tag @AaronBallman, since this PR fixes a bug that was introduced when octal prefixes were added in #131626. |
Fixes llvm#141230. Currently, prefixed octal literals used with floating-point suffixes are not rejected, causing Clang to crash. This adds proper handling to reject invalid literals such as `0o0.1` or `0.0e1`. No release note because this is fixing an issue with a new change.
naveen-seth
force-pushed
the
fix-n3353-float-suffix
branch
from
9773316 to
8556e25
Compare
There was a problem hiding this comment.
I added @cor3ntin and @tahonermann for some extra sets of eyes, seeing as how I already messed this up once before. :-)
| @@ -1432,14 +1432,26 @@ void NumericLiteralParser::ParseNumberStartingWithZero(SourceLocation TokLoc) { | |||
| Diags.Report(TokLoc, DiagId); | |||
| ++s; | |||
| DigitsBegin = s; | |||
| SawOctalPrefix = true; | |||
| radix = 8; | |||
There was a problem hiding this comment.
There's code below which also unconditionally sets radix to 8; we should remove that and update the comments. We also seem to skip the octal digits twice (I think it's a noop to do it a second time, but it's still a code smell).
There was a problem hiding this comment.
I don't think the octal digits are skipped twice here, because the function now always returns before exiting the if-block we are currently in.
llvm-project/clang/lib/Lex/LiteralSupport.cpp
Lines 1447 to 1448 in 8556e25
This separates parsing for octals with a literal prefix from those starting with
0, so that the logic for potentially parsing a floating-point only applies to the latter (otherwise, we get this crash).Let me know if I am missing something here!
For the radix assignment, this could be shared between both cases if we move it above the current if statement.
There was a problem hiding this comment.
Oh, you're right, I missed the unconditional return!
Fixes #141230.
Currently, prefixed octal literals used with floating-point suffixes are not
rejected, causing Clang to crash.
This adds proper handling to reject invalid literals such as
0o0.1or0.0e1.No release note because this is fixing an issue with a new change.