llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Akash Manna (akash-manna-sky) <details> <summary>Changes</summary> Fixes #<!-- -->209503 `ObjCImplParsingDataRAII` only reset `CurParsedObjCImpl` in its destructor, but an `@<!-- -->implementation` can be finished well before that: `CheckNestedObjCContexts` ends it as soon as a nested `@<!-- -->interface`/`@<!-- -->protocol`/`@<!-- -->implementation` shows up, and that container is then parsed inside the same frame. Anything defined in there still looked like it was inside the already finished `@<!-- -->implementation`, so `ParseFunctionDefinition` queued the body into a `LateParsedObjCMethods` that nobody was going to drain again, and the destructor's `assert(LateParsedObjCMethods.empty())` fired. The fatal `#include` in the report is a red herring, by the way — it only hides the diagnostics for the function that gets queued. `finish()` now restores the previous `CurParsedObjCImpl` the moment the `@<!-- -->implementation` ends, instead of the destructor nulling it later. Restoring rather than clearing also covers the mirror image: an `@<!-- -->implementation` nested through a namespace inside another one used to wipe out the outer, still open one when it ended, and the next method definition in the outer one tripped the `Method out of @<!-- -->implementation` assert. The new test has both shapes. --- Full diff: https://github.com/llvm/llvm-project/pull/219953.diff 4 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+3) - (modified) clang/include/clang/Parse/Parser.h (+3-1) - (modified) clang/lib/Parse/ParseObjc.cpp (+4-1) - (added) clang/test/Parser/GH209503.mm (+27) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index bdbabf2cd98d0..598b7cebd15d0 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -573,6 +573,9 @@ features cannot lower the translation-unit ABI level; threshold to the target's `size_t` width instead of using a fixed threshold of `1 << 60` regardless of the target. - Fixed a crash when generating fake uses for parameters of bodyless destructors with `-fextend-variable-liveness`. +- Fixed an assertion failure when a method or function definition follows an + Objective-C `@implementation` that was ended by a nested `@interface`, + `@protocol` or `@implementation` before its `@end`. (#GH209503) ### OpenACC Specific Changes diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index ae91153e34e3a..8b5a0ad732ece 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -5710,7 +5710,8 @@ class Parser : public CodeCompletionHandler { LateParsedObjCMethodContainer LateParsedObjCMethods; ObjCImplParsingDataRAII(Parser &parser, Decl *D) - : P(parser), Dcl(D), HasCFunction(false) { + : P(parser), Dcl(D), HasCFunction(false), + PrevParsedObjCImpl(parser.CurParsedObjCImpl) { P.CurParsedObjCImpl = this; Finished = false; } @@ -5720,6 +5721,7 @@ class Parser : public CodeCompletionHandler { bool isFinished() const { return Finished; } private: + ObjCImplParsingDataRAII *PrevParsedObjCImpl; bool Finished; }; ObjCImplParsingDataRAII *CurParsedObjCImpl; diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp index d01b0abf917cb..40d4aca43047c 100644 --- a/clang/lib/Parse/ParseObjc.cpp +++ b/clang/lib/Parse/ParseObjc.cpp @@ -2035,7 +2035,6 @@ Parser::ObjCImplParsingDataRAII::~ObjCImplParsingDataRAII() { << SemaObjC::OCK_Implementation; } } - P.CurParsedObjCImpl = nullptr; assert(LateParsedObjCMethods.empty()); } @@ -2061,6 +2060,10 @@ void Parser::ObjCImplParsingDataRAII::finish(SourceRange AtEnd) { delete *I; LateParsedObjCMethods.clear(); + // Parsing may go on in the enclosing frame before this object is destroyed + // (e.g. a nested @interface ended the @implementation early), so stop being + // the current @implementation now rather than in the destructor. + P.CurParsedObjCImpl = PrevParsedObjCImpl; Finished = true; } diff --git a/clang/test/Parser/GH209503.mm b/clang/test/Parser/GH209503.mm new file mode 100644 index 0000000000000..43d14b242dbcc --- /dev/null +++ b/clang/test/Parser/GH209503.mm @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s + +// Method and function bodies are parsed when their @implementation ends; make +// sure they are queued into the right @implementation (if any) after a nested +// container has ended one. + +@interface Z +@end +@interface A +@end + +@implementation Z +namespace N { +@implementation A // expected-error {{Objective-C declarations may only appear in global scope}} +@end +} +- (void)m { + undeclared(); // expected-error {{use of undeclared identifier 'undeclared'}} +} +@end + +@implementation NSArray // expected-warning {{cannot find interface declaration for 'NSArray'}} \ + // expected-note {{implementation started here}} +@interface NSIndexSet // expected-error {{missing '@end'}} \ + // expected-note {{class started here}} +// expected-warning@+1 {{function definition inside an Objective-C container is deprecated}} +void f(void) {} // expected-error {{missing '@end'}} `````````` </details> https://github.com/llvm/llvm-project/pull/219953 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
