[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288893: [ObjC++] Don't enter a C++ declarator scope when the
current context is (authored by arphaman).
Changed prior to commit:
https://reviews.llvm.org/D26922?vs=79722=80556#toc
Repository:
rL LLVM
https://reviews.llvm.org/D26922
Files:
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
cfe/trunk/test/SemaObjCXX/crash.mm
Index: cfe/trunk/test/SemaObjCXX/crash.mm
===
--- cfe/trunk/test/SemaObjCXX/crash.mm
+++ cfe/trunk/test/SemaObjCXX/crash.mm
@@ -25,3 +25,38 @@
// expected-warning@-2 {{variadic templates are a C++11 extension}}
#endif
@end
+
+// rdar://20560175
+
+struct OuterType {
+ typedef int InnerType;
+};
+
+namespace ns {
+ typedef int InnerType;
+};
+
+@protocol InvalidProperties
+
+@property (nonatomic) (OuterType::InnerType) invalidTypeParens;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) (ns::InnerType) invalidTypeParens2;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) int OuterType::InnerType; // expected-error {{property
requires fields to be named}}
+
+@property (nonatomic) int OuterType::InnerType foo; // expected-error
{{property requires fields to be named}}
+// expected-error@-1 {{expected ';' at end of declaration list}}
+// expected-error@-2 {{C++ requires a type specifier for all declarations}}
+// expected-error@-3 {{cannot declare variable inside @interface or @protocol}}
+
+@end
Index: cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
===
--- cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
+++ cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
@@ -1001,6 +1001,11 @@
bool Sema::ShouldEnterDeclaratorScope(Scope *S, const CXXScopeSpec ) {
assert(SS.isSet() && "Parser passed invalid CXXScopeSpec.");
+ // Don't enter a declarator context when the current context is an
Objective-C
+ // declaration.
+ if (isa(CurContext) || isa(CurContext))
+return false;
+
NestedNameSpecifier *Qualifier = SS.getScopeRep();
// There are only two places a well-formed program may qualify a
Index: cfe/trunk/lib/Parse/ParseDecl.cpp
===
--- cfe/trunk/lib/Parse/ParseDecl.cpp
+++ cfe/trunk/lib/Parse/ParseDecl.cpp
@@ -5264,6 +5264,14 @@
// Change the declaration context for name lookup, until this function
// is exited (and the declarator has been parsed).
DeclScopeObj.EnterDeclaratorScope();
+ else if (getObjCDeclContext()) {
+// Ensure that we don't interpret the next token as an identifier when
+// dealing with declarations in an Objective-C container.
+D.SetIdentifier(nullptr, Tok.getLocation());
+D.setInvalidType(true);
+ConsumeToken();
+goto PastIdentifier;
+ }
}
// C++0x [dcl.fct]p14:
Index: cfe/trunk/test/SemaObjCXX/crash.mm
===
--- cfe/trunk/test/SemaObjCXX/crash.mm
+++ cfe/trunk/test/SemaObjCXX/crash.mm
@@ -25,3 +25,38 @@
// expected-warning@-2 {{variadic templates are a C++11 extension}}
#endif
@end
+
+// rdar://20560175
+
+struct OuterType {
+ typedef int InnerType;
+};
+
+namespace ns {
+ typedef int InnerType;
+};
+
+@protocol InvalidProperties
+
+@property (nonatomic) (OuterType::InnerType) invalidTypeParens;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) (ns::InnerType) invalidTypeParens2;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) int OuterType::InnerType; // expected-error {{property
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
ahatanak accepted this revision. ahatanak added a comment. This revision is now accepted and ready to land. I think this is fine. I guess we can print a more helpful error message than "error: property requires fields to be named", but we can probably do it later. Repository: rL LLVM https://reviews.llvm.org/D26922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
arphaman added a comment. > Also, the following code (which is not valid) crashes with ToT trunk, > > @property (nonatomic) int OuterType::InnerType > > > but compiles without any errors with your patch applied. Do you know why > clang doesn't error out? This worked because clang continued parsing `InnerType` as the name of the property. The updated patch fixes this by ensuring that we don't interpret this token as a property name, so now this example will have a compilation error. Repository: rL LLVM https://reviews.llvm.org/D26922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
arphaman updated this revision to Diff 79722.
arphaman added a comment.
The updated patch performs the check in `ShouldEnterDeclaratorScope`.
Repository:
rL LLVM
https://reviews.llvm.org/D26922
Files:
lib/Parse/ParseDecl.cpp
lib/Sema/SemaCXXScopeSpec.cpp
test/SemaObjCXX/crash.mm
Index: test/SemaObjCXX/crash.mm
===
--- test/SemaObjCXX/crash.mm
+++ test/SemaObjCXX/crash.mm
@@ -25,3 +25,38 @@
// expected-warning@-2 {{variadic templates are a C++11 extension}}
#endif
@end
+
+// rdar://20560175
+
+struct OuterType {
+ typedef int InnerType;
+};
+
+namespace ns {
+ typedef int InnerType;
+};
+
+@protocol InvalidProperties
+
+@property (nonatomic) (OuterType::InnerType) invalidTypeParens;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) (ns::InnerType) invalidTypeParens2;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) int OuterType::InnerType; // expected-error {{property
requires fields to be named}}
+
+@property (nonatomic) int OuterType::InnerType foo; // expected-error
{{property requires fields to be named}}
+// expected-error@-1 {{expected ';' at end of declaration list}}
+// expected-error@-2 {{C++ requires a type specifier for all declarations}}
+// expected-error@-3 {{cannot declare variable inside @interface or @protocol}}
+
+@end
Index: lib/Sema/SemaCXXScopeSpec.cpp
===
--- lib/Sema/SemaCXXScopeSpec.cpp
+++ lib/Sema/SemaCXXScopeSpec.cpp
@@ -1001,6 +1001,11 @@
bool Sema::ShouldEnterDeclaratorScope(Scope *S, const CXXScopeSpec ) {
assert(SS.isSet() && "Parser passed invalid CXXScopeSpec.");
+ // Don't enter a declarator context when the current context is an
Objective-C
+ // declaration.
+ if (isa(CurContext) || isa(CurContext))
+return false;
+
NestedNameSpecifier *Qualifier = SS.getScopeRep();
// There are only two places a well-formed program may qualify a
Index: lib/Parse/ParseDecl.cpp
===
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -5264,6 +5264,14 @@
// Change the declaration context for name lookup, until this function
// is exited (and the declarator has been parsed).
DeclScopeObj.EnterDeclaratorScope();
+ else if (getObjCDeclContext()) {
+// Ensure that we don't interpret the next token as an identifier when
+// dealing with declarations in an Objective-C container.
+D.SetIdentifier(nullptr, Tok.getLocation());
+D.setInvalidType(true);
+ConsumeToken();
+goto PastIdentifier;
+ }
}
// C++0x [dcl.fct]p14:
Index: test/SemaObjCXX/crash.mm
===
--- test/SemaObjCXX/crash.mm
+++ test/SemaObjCXX/crash.mm
@@ -25,3 +25,38 @@
// expected-warning@-2 {{variadic templates are a C++11 extension}}
#endif
@end
+
+// rdar://20560175
+
+struct OuterType {
+ typedef int InnerType;
+};
+
+namespace ns {
+ typedef int InnerType;
+};
+
+@protocol InvalidProperties
+
+@property (nonatomic) (OuterType::InnerType) invalidTypeParens;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) (ns::InnerType) invalidTypeParens2;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{property requires fields to be named}}
+// expected-error@-3 {{expected ';' at end of declaration list}}
+// expected-error@-4 {{C++ requires a type specifier for all declarations}}
+// expected-error@-5 {{cannot declare variable inside @interface or @protocol}}
+
+@property (nonatomic) int OuterType::InnerType; // expected-error {{property requires fields to be named}}
+
+@property (nonatomic) int OuterType::InnerType foo; // expected-error {{property requires fields to be named}}
+// expected-error@-1 {{expected ';' at end of declaration list}}
+// expected-error@-2 {{C++ requires a type specifier for all declarations}}
+//
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
arphaman added a comment. In https://reviews.llvm.org/D26922#608815, @ahatanak wrote: > Yes, I meant ParseDecl:5266. Reading the comment in > ShouldEnterDeclaratorScope, it seemed to me that it shouldn't return true in > this context (when parsing an objective-c). Yes, I agree that it's better to move the check to ShouldEnterDeclaratorScope, I will update the patch. > Also, the following code (which is not valid) crashes with ToT trunk, > > @property (nonatomic) int OuterType::InnerType > > > but compiles without any errors with your patch applied. Do you know why > clang doesn't error out? Repository: rL LLVM https://reviews.llvm.org/D26922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
arphaman added a comment. In https://reviews.llvm.org/D26922#608815, @ahatanak wrote: > Yes, I meant ParseDecl:5266. Reading the comment in > ShouldEnterDeclaratorScope, it seemed to me that it shouldn't return true in > this context (when parsing an objective-c). > > Also, the following code (which is not valid) crashes with ToT trunk, > > @property (nonatomic) int OuterType::InnerType > > > but compiles without any errors with your patch applied. Do you know why > clang doesn't error out? Hmm, that's interesting, I have to investigate this. It looks like clang is creating a property named `InnerType` which is obviously invalid. Repository: rL LLVM https://reviews.llvm.org/D26922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
ahatanak added a comment. Yes, I meant ParseDecl:5266. Reading the comment in ShouldEnterDeclaratorScope, it seemed to me that it shouldn't return true in this context (when parsing an objective-c). Also, the following code (which is not valid) crashes with ToT trunk, @property (nonatomic) int OuterType::InnerType but compiles without any errors with your patch applied. Do you know why clang doesn't error out? Repository: rL LLVM https://reviews.llvm.org/D26922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
arphaman added a comment.
In https://reviews.llvm.org/D26922#607186, @ahatanak wrote:
> I wonder whether it is possible to avoid calling
> DeclScopeObj.EnterDeclaratorScope at ParseDecl.cpp:5317 instead of fixing
> Sema::ActOnCXXEnterDeclaratorScope?
>
> I believe it's calling EnterDeclaratorScope to enable name lookup when a
> namespace-member variable is defined outside the namespace. Since that's not
> the case here, it seems to me that it shouldn't called in the first place.
>
> http://en.cppreference.com/w/cpp/language/unqualified_lookup
Do you mean at ParseDecl:5266 (I get this line in the crash backtrace with ToT)?
AFAIK it's also used for static class member lookups for variables defined
outside of a class. The `OuterType` is a C++ record so that's why it should be
valid to call `EnterDeclaratorScope` there. I tested to verify this and I reach
that call on line 5266 with the following example 3 times:
namespace Foo {
extern int x;
}
int Foo::x = 2; // EnterDeclaratorScope is called here
class Bar {
public:
void foo();
static int m;
};
int Bar::m = 2; // here
class FooBar {
friend void Bar::foo(); // and here
};
So it doesn't seem reasonable to required this only for namespaces.
I suppose it might be better to avoid calling `EnterDeclaratorScope` anyway by
moving the current context ObjC isa checks to `ShouldEnterDeclaratorScope`.
Repository:
rL LLVM
https://reviews.llvm.org/D26922
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
ahatanak added a comment. I wonder whether it is possible to avoid calling DeclScopeObj.EnterDeclaratorScope at ParseDecl.cpp:5317 instead of fixing Sema::ActOnCXXEnterDeclaratorScope? I believe it's calling EnterDeclaratorScope to enable name lookup when a namespace-member variable is defined outside the namespace. Since that's not the case here, it seems to me that it shouldn't called in the first place. http://en.cppreference.com/w/cpp/language/unqualified_lookup Repository: rL LLVM https://reviews.llvm.org/D26922 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26922: [ObjC++] Don't enter a C++ declarator context when the current context is an Objective-C declaration
arphaman created this revision.
arphaman added reviewers: manmanren, ahatanak.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.
This patch ensures that Sema won't enter a C++ declarator context when the
current context is an Objective-C declaration. This prevents an assertion
failure in `EnterDeclaratorContext` that's used to ensure that current context
will be restored correctly after exiting the declarator context. I think that
this approach is reasonable as AFAIK we don't need to use the C++ declarator
contexts directly in Objective-C declarations, since they're mostly used to
define out of line variables declared in classes or namespaces which shouldn't
be done inside an Objective-C declaration.
Repository:
rL LLVM
https://reviews.llvm.org/D26922
Files:
lib/Sema/SemaCXXScopeSpec.cpp
test/SemaObjCXX/crash.mm
Index: test/SemaObjCXX/crash.mm
===
--- test/SemaObjCXX/crash.mm
+++ test/SemaObjCXX/crash.mm
@@ -25,3 +25,17 @@
// expected-warning@-2 {{variadic templates are a C++11 extension}}
#endif
@end
+
+// rdar://20560175
+
+struct OuterType {
+ typedef int InnerType;
+};
+
+@protocol InvalidProperty
+@property (nonatomic) (OuterType::InnerType) invalidTypeParens;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{expected ';' at end of declaration list}}
+// expected-error@-3 {{C++ requires a type specifier for all declarations}}
+// expected-error@-4 {{cannot declare variable inside @interface or @protocol}}
+@end
Index: lib/Sema/SemaCXXScopeSpec.cpp
===
--- lib/Sema/SemaCXXScopeSpec.cpp
+++ lib/Sema/SemaCXXScopeSpec.cpp
@@ -1054,7 +1054,12 @@
// it is a complete declaration context.
if (!DC->isDependentContext() && RequireCompleteDeclContext(SS, DC))
return true;
-
+
+ // Don't enter a declarator context when the current context is an
Objective-C
+ // declaration as we might no be able to restore it when exiting the scope.
+ if (isa(CurContext) || isa(CurContext))
+return true;
+
EnterDeclaratorContext(S, DC);
// Rebuild the nested name specifier for the new scope.
Index: test/SemaObjCXX/crash.mm
===
--- test/SemaObjCXX/crash.mm
+++ test/SemaObjCXX/crash.mm
@@ -25,3 +25,17 @@
// expected-warning@-2 {{variadic templates are a C++11 extension}}
#endif
@end
+
+// rdar://20560175
+
+struct OuterType {
+ typedef int InnerType;
+};
+
+@protocol InvalidProperty
+@property (nonatomic) (OuterType::InnerType) invalidTypeParens;
+// expected-error@-1 {{type name requires a specifier or qualifier}}
+// expected-error@-2 {{expected ';' at end of declaration list}}
+// expected-error@-3 {{C++ requires a type specifier for all declarations}}
+// expected-error@-4 {{cannot declare variable inside @interface or @protocol}}
+@end
Index: lib/Sema/SemaCXXScopeSpec.cpp
===
--- lib/Sema/SemaCXXScopeSpec.cpp
+++ lib/Sema/SemaCXXScopeSpec.cpp
@@ -1054,7 +1054,12 @@
// it is a complete declaration context.
if (!DC->isDependentContext() && RequireCompleteDeclContext(SS, DC))
return true;
-
+
+ // Don't enter a declarator context when the current context is an Objective-C
+ // declaration as we might no be able to restore it when exiting the scope.
+ if (isa(CurContext) || isa(CurContext))
+return true;
+
EnterDeclaratorContext(S, DC);
// Rebuild the nested name specifier for the new scope.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
