Skip to content

[C2y] Handle FP-suffixes on prefixed octals (#141230) #141695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+38 −4
Open

[C2y] Handle FP-suffixes on prefixed octals (#141230) #141695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions clang/lib/Lex/LiteralSupport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -1432,14 +1432,26 @@ void NumericLiteralParser::ParseNumberStartingWithZero(SourceLocation TokLoc) {
Diags.Report(TokLoc, DiagId);
++s;
DigitsBegin = s;
SawOctalPrefix = true;
radix = 8;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// Other suffixes will be diagnosed by the caller.
return;

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're right, I missed the unconditional return!

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);
});

Expand Down
22 changes: 22 additions & 0 deletions clang/test/C/C2y/n3353.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
Loading