Sylvestre Ledru [:Sylvestre] Assignee
	Description • 7 years ago

/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,

	Nathan Froyd [:froydnj]
	Comment 2 • 7 years ago
mozreview-review

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.

	(Away)
	Comment 3 • 7 years ago

> 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.

	Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)
	Comment 4 • 7 years ago

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))`.

	James Cheng[:JamesCheng]
	Comment 5 • 7 years ago

(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))`.

	Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)
	Comment 7 • 7 years ago
mozreview-review

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.

	Pulsebot
	Comment 8 • 7 years ago

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

	(Away)
	Comment 9 • 7 years ago

Please update the assertion string too...

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 10 • 7 years ago

Sure, will do.

	Noemi Erli[:noemi_erli]
	Comment 11 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b2de74e80df2

	Sylvestre Ledru [:Sylvestre] Assignee
	Comment 12 • 7 years ago

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

	Andreea Pavel [:apavel]
	Comment 13 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/ce69a5d9a56c