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.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
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
Comment Actions
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.
Comment Actions
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?
Comment Actions
Hmm, that's interesting, I have to investigate this. It looks like clang is creating a property named InnerType which is obviously invalid.
Comment Actions
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::InnerTypebut compiles without any errors with your patch applied. Do you know why clang doesn't error out?
Comment Actions
Also, the following code (which is not valid) crashes with ToT trunk,
@property (nonatomic) int OuterType::InnerTypebut 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.
Comment Actions
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.