Skip to content

Fix crash with invalid member function param list #139595

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

Merged
merged 2 commits into from
May 13, 2025
Merged

Fix crash with invalid member function param list #139595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e43846
Fix crash with invalid member function param list
AaronBallman May 12, 2025
c688822
Merge branch 'main' into aballman-gh113722
AaronBallman May 13, 2025
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
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,8 @@ Bug Fixes in This Version
``#include`` directive. (#GH138094)
- Fixed a crash during constant evaluation involving invalid lambda captures
(#GH138832)
- Fixed a crash with an invalid member function parameter list with a default
argument which contains a pragma. (#GH113722)
- Fixed assertion failures when generating name lookup table in modules. (#GH61065, #GH134739)

Bug Fixes to Compiler Builtins
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/Parse/ParseCXXInlineMethods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,10 +1272,6 @@ bool Parser::ConsumeAndStoreInitializer(CachedTokens &Toks,
goto consume_token;

case tok::eof:
case tok::annot_module_begin:
case tok::annot_module_end:
case tok::annot_module_include:
case tok::annot_repl_input_end:
// Ran out of tokens.
return false;

Expand Down Expand Up @@ -1411,6 +1407,11 @@ bool Parser::ConsumeAndStoreInitializer(CachedTokens &Toks,
[[fallthrough]];
default:
consume_token:
// If it's an annotation token, then we've run out of tokens and should
// bail out. Otherwise, cache the token and consume it.
if (Tok.isAnnotation())
return false;

Toks.push_back(Tok);
ConsumeToken();
break;
Expand Down
10 changes: 10 additions & 0 deletions clang/test/Parser/cxx-invalid-function-decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,13 @@ struct S : public Base1<int>, public Base2<float> {
// All initializers are correct, nothing to skip, diagnose 2 missing commas.
S(const S &) : Base1<int>(0) ::Base2<float>(1.0) x(2) {} // expected-error2{{missing ',' between base or member initializers}}
};

namespace GH113722 {
struct S {
void m(int x = 0; // expected-error {{unexpected end of default argument expression}} \
expected-note {{to match this '('}}
#pragma unused(x) // expected-error {{expected ')'}} \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this mean we can't have pragmas in the middle of a statement? It seems that something like the above is actually useful/a shame to lose. It would be wonderful if instead the ConsumeAndStoreInitializer just 'did the pragma parsing and came back', but I presume that is a lot more work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Does this mean we can't have pragmas in the middle of a statement?

Nope, this still works fine:

struct S {
  void m(int x =
  #pragma clang diagnostic push
  #pragma clang diagnostic ignored "-Wconversion"
  1.2f
  #pragma clang diagnostic pop
  );
  void n(int x =
  1.2f
  );
};

I can add a test case for that if you want. This is just an issue with an ill-formed initializer, I believe.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OH! missed that this was a function parameter/default argument and part of one that is already ill-formed. I thinks this recovery makes sense, I don't see any need for other tests

expected-error {{expected ';' at end of declaration list}}
} // expected-error {{expected ';' after struct}}
};
} // expected-error {{extraneous closing brace ('}')}}
Loading