Closed Bug 1424866 Opened 2 years ago Closed 2 years ago

TestTypeTraits.cpp:74:30: error: unnecessary parentheses in declaration of 'type name'

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: Sylvestre, Assigned: Sylvestre)

References

(Depends on 1 open bug)

Details

Attachments

(1 file)

/root/firefox-gcc-last/mfbt/tests/TestTypeTraits.cpp:74:30: error: unnecessary parentheses in declaration of 'type name' [-Werror=parentheses]
  static_assert(!IsPointer<bool(IsPointerTest::*)>::value,
Assignee: nobody → sledru
Comment on attachment 8936376 [details]
Bug 1424866 - Fix a warning: unnecessary parentheses in declaration of 'type name'

https://reviewboard.mozilla.org/r/207102/#review212952

Congratulations on finding your way into my review queue! :)  Comments below.

::: mfbt/tests/TestTypeTraits.cpp:74
(Diff revision 1)
> -static_assert(!IsPointer<bool(IsPointerTest::*)>::value,
> +static_assert(!IsPointer<bool>::value,
>                "bool(IsPointerTest::*) not a pointer");

This doesn't seem quite right--at a minimum, the assert message would need to be changed.

I think we're trying to test that pointers to members are not pointers...but I guess the parentheses screw that up, and we're actually testing coercion here?

It seems the right thing here is `bool IsPointerTest::*`...but I don't know if that actually works with the test as written.
Attachment #8936376 - Flags: review?(nfroyd) → review-
> It seems the right thing here is `bool IsPointerTest::*`...but I don't know
> if that actually works with the test as written.

That ought to work, and it seems to match the intent given by the comment blurb at IsPointer.

It would be good to also patch up the example at https://searchfox.org/mozilla-central/source/mfbt/TypeTraits.h#229.
Looks like I wrote this. Sorry!

As Nathan guessed in comment 2, `bool(IsPointerTest::*)` was supposed to represent a pointer to a member function that returns bool (for which IsPointer should be false), aren't the parentheses necessary?
There's another similar test just below, for a pointer to a member function that returns void.

I don't have gcc 8 handy, could you please try: `using PointerToMemberReturningBool = bool(IsPointerTest::*); static_assert(!IsPointer<PointerToMemberReturningBool>::value, "...");`
If that still doesn't work, you could try adding `int i();` to the IsPointerTest struct, and then test `IsPointer(decltype(IsPointerTest::i))`.
(In reply to Gerald Squelart [:gerald] from comment #4)
> Looks like I wrote this. Sorry!
> 
> As Nathan guessed in comment 2, `bool(IsPointerTest::*)` was supposed to
> represent a pointer to a member function that returns bool 

For member function,
should it be "bool(IsPointerTest::*)()" instead? 

(for which
> IsPointer should be false), aren't the parentheses necessary?
> There's another similar test just below, for a pointer to a member function
> that returns void.
> 
> I don't have gcc 8 handy, could you please try: `using
> PointerToMemberReturningBool = bool(IsPointerTest::*);
> static_assert(!IsPointer<PointerToMemberReturningBool>::value, "...");`
> If that still doesn't work, you could try adding `int i();` to the
> IsPointerTest struct, and then test `IsPointer(decltype(IsPointerTest::i))`.
Comment on attachment 8936376 [details]
Bug 1424866 - Fix a warning: unnecessary parentheses in declaration of 'type name'

https://reviewboard.mozilla.org/r/207102/#review215552

Well, duh! Silly me for missing this. :-P
Thanks James and Sylvestre.
Attachment #8936376 - Flags: review?(gsquelart) → review+
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2de74e80df2
Fix a warning: unnecessary parentheses in declaration of 'type name' r=gerald
Please update the assertion string too...
Sure, will do.
https://hg.mozilla.org/mozilla-central/rev/b2de74e80df2
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce69a5d9a56cb4425ec96c38e0dfbe3e54cf5fbe
Bug 1424866 - Update the comment for fix a warning: unnecessary parentheses in declaration of 'type name' r=gerald DONTBUILD
Depends on: 1428005
You need to log in before you can comment on or make changes to this bug.