Objective-C++11: concerns about adding static_assert support to @interface / @implementation?

Hi,

I’m trying to enable -Wextra-semi in Chromium. I’m almost there; the last remaining issue is that we have a macro THREAD_CHECKER(name) that expands to nothing in release builds but to ThreadChecker name in debug builds. It’s supposed to be used like

class C {
THREAD_CHECKER(checker_);
};

And then in the implementation, you do CHECK_CALLED_ON_VALID_THREAD(checker_); which again does nothing in release builds but calls a method on checker_ in debug builds.

Since this is a declaration, this triggers -Wextra-semi. I figured the way to spell the usual do {} while (0) you tag on the end of a statement macro to silence this warning could be static_assert(1, ""), so I changed THREAD_CHECKER() to expand to that in release builds instead of nothing.

This works beautifully, but Objective-C interfaces don’t support static_assert()s as I have learned. So I’d like to add support for that.

Before I start writing a patch for that: Are people fine with supporting static_assert in the member list of an @interface or @implementation in Obj-C? It not being supported seems like an oversight to me, but maybe I’m missing something – so I figured I’d ask before I start writing a patch :slight_smile:

Nico

Hi,

I'm trying to enable -Wextra-semi in Chromium. I'm almost there; the last remaining issue is that we have a macro THREAD_CHECKER(name) that expands to nothing in release builds but to `ThreadChecker name` in debug builds. It's supposed to be used like

class C {
  THREAD_CHECKER(checker_);
};

And then in the implementation, you do CHECK_CALLED_ON_VALID_THREAD(checker_); which again does nothing in release builds but calls a method on checker_ in debug builds.

Since this is a declaration, this triggers -Wextra-semi. I figured the way to spell the usual `do {} while (0)` you tag on the end of a statement macro to silence this warning could be `static_assert(1, "")`, so I changed THREAD_CHECKER() to expand to that in release builds instead of nothing.

This works beautifully, but Objective-C interfaces don't support static_assert()s as I have learned. So I'd like to add support for that.

Do they support _Static_assert() instead (the C11 spelling), and only
support static_assert() if you #include <assert.h> first?

~Aaron

No.

For what little it’s worth, my opinion is that supporting static_assert/_Static_assert in Objective-C++/Objective-C sounds like a great idea.

However, if you need something for Chromium that works right now and in previous versions of Clang too, have you considered

#define THREAD_CHECKER(name) friend void __dummy()

or

#define THREAD_CHECKER(name) static constexpr int __dummy = 0

The latter should work in most places (as long as you only need one per scope, or you could do __COUNTER__ tricks if you need several in the same scope). The former should work as long as you only ever need it in class bodies.
(Bikeshedding the appropriate spelling of __dummy is left as an exercise for Chromium’s maintainers.)

–Arthur

Hi,

I’m trying to enable -Wextra-semi in Chromium. I’m almost there; the last remaining issue is that we have a macro THREAD_CHECKER(name) that expands to nothing in release builds but to ThreadChecker name in debug builds. It’s supposed to be used like

class C {
THREAD_CHECKER(checker_);
};

[…] Since this is a declaration, this triggers -Wextra-semi. I figured the way to spell the usual do {} while (0) you tag on the end of a statement macro to silence this warning could be static_assert(1, ""), so I changed THREAD_CHECKER() to expand to that in release builds instead of nothing.

For what little it’s worth, my opinion is that supporting static_assert/_Static_assert in Objective-C++/Objective-C sounds like a great idea.

However, if you need something for Chromium that works right now and in previous versions of Clang too, have you considered

Thanks for the suggestion! I see I didn’t explain this very well; for Obj-C we have an Obj-C class that does

@interface A {
THREAD_CHECKER(checker_);
}
@end

and static_assert() isn’t supported there. (Neither are friend functions nor static constexpr fields, since Objective-C doesn’t have the friend syntax or static instance variables at class scope).

+1 from me, it seems like it would be useful to support _Static_assert there.

Erik

Good to know!

I'd also like to see _Static_assert() supported in Objective-C and
static_assert() supported in Objective-C++; I think they'd be useful.
However, I'm not certain of the relationship between ObjC/C++ versions
and the language standard version of C/C++ and whether there might be
concerns there.

~Aaron

For what it's worth, I have successfully used static_assert in Objective-C++ code in Xcode 10.1. I don't know whether the fact that it's Apple Clang is relevant.

Hi,

I'm trying to enable -Wextra-semi in Chromium. I'm almost there; the last remaining issue is that we have a macro THREAD_CHECKER(name) that expands to nothing in release builds but to `ThreadChecker name` in debug builds. It's supposed to be used like

class C {
THREAD_CHECKER(checker_);
};

And then in the implementation, you do CHECK_CALLED_ON_VALID_THREAD(checker_); which again does nothing in release builds but calls a method on checker_ in debug builds.

Since this is a declaration, this triggers -Wextra-semi. I figured the way to spell the usual `do {} while (0)` you tag on the end of a statement macro to silence this warning could be `static_assert(1, "")`, so I changed THREAD_CHECKER() to expand to that in release builds instead of nothing.

This works beautifully, but Objective-C interfaces don't support static_assert()s as I have learned. So I'd like to add support for that.

Do they support _Static_assert() instead (the C11 spelling)

No.

Good to know!

I'd also like to see _Static_assert() supported in Objective-C and
static_assert() supported in Objective-C++; I think they'd be useful.
However, I'm not certain of the relationship between ObjC/C++ versions
and the language standard version of C/C++ and whether there might be
concerns there.

My understanding is that we try to be as compatible as possible with the base language, so -xobjective-c++ -std=c++11 should behave just like -std=c++11, but also have objective-c stuff. Thats what we do in general with static_assert/_Static_assert here too, its just that for some reason we don’t parse them in this context.

Erik

>
>>
>>>
>>>>
>>>> Hi,
>>>>
>>>> I'm trying to enable -Wextra-semi in Chromium. I'm almost there; the last remaining issue is that we have a macro THREAD_CHECKER(name) that expands to nothing in release builds but to `ThreadChecker name` in debug builds. It's supposed to be used like
>>>>
>>>> class C {
>>>> THREAD_CHECKER(checker_);
>>>> };
>>>>
>>>> And then in the implementation, you do CHECK_CALLED_ON_VALID_THREAD(checker_); which again does nothing in release builds but calls a method on checker_ in debug builds.
>>>>
>>>> Since this is a declaration, this triggers -Wextra-semi. I figured the way to spell the usual `do {} while (0)` you tag on the end of a statement macro to silence this warning could be `static_assert(1, "")`, so I changed THREAD_CHECKER() to expand to that in release builds instead of nothing.
>>>>
>>>> This works beautifully, but Objective-C interfaces don't support static_assert()s as I have learned. So I'd like to add support for that.
>>>
>>> Do they support _Static_assert() instead (the C11 spelling)
>>
>>
>> No.
>
> Good to know!
>
> I'd also like to see _Static_assert() supported in Objective-C and
> static_assert() supported in Objective-C++; I think they'd be useful.
> However, I'm not certain of the relationship between ObjC/C++ versions
> and the language standard version of C/C++ and whether there might be
> concerns there.

My understanding is that we try to be as compatible as possible with the base language, so -xobjective-c++ -std=c++11 should behave just like -std=c++11, but also have objective-c stuff. Thats what we do in general with static_assert/_Static_assert here too, its just that for some reason we don’t parse them in this context.

Ah, nice to know, thank you Erik. Then yeah, count me as a +1 for
supporting this.

~Aaron

Patch: https://reviews.llvm.org/D59223