[clang-format] Misformatted address-of operator after cast on 20 versus 19

[rmarker]

While testing on the release branch for 20, I discovered this formatting change from 19.1.0.

Using 19.1.0:

::gluTessCallback(tesselator.get(), GLU_TESS_COMBINE_DATA,
                  (GLvoid(FONM_GLU_STD_CALLBACK *)())&CallbackCombine);

Using 20 (8c25748):

::gluTessCallback(tesselator.get(), GLU_TESS_COMBINE_DATA,
                  (GLvoid(FONM_GLU_STD_CALLBACK *)()) & CallbackCombine);

[llvmbot]

@llvm/issue-subscribers-clang-format

Author: None (rmarker)

While testing on the release branch for 20, I discovered this formatting change from 19.1.0.

Using 19.1.0:

::gluTessCallback(tesselator.get(), GLU_TESS_COMBINE_DATA,
                  (GLvoid(FONM_GLU_STD_CALLBACK *)())&CallbackCombine);

Using 20 (8c25748):

::gluTessCallback(tesselator.get(), GLU_TESS_COMBINE_DATA,
                  (GLvoid(FONM_GLU_STD_CALLBACK *)()) & CallbackCombine);

[owenca]

While testing on the release branch for 20, I discovered this formatting change from 19.1.0.

Using 19.1.0:

::gluTessCallback(tesselator.get(), GLU_TESS_COMBINE_DATA,
(GLvoid(FONM_GLU_STD_CALLBACK *)())&CallbackCombine);
Using 20 (8c25748):

::gluTessCallback(tesselator.get(), GLU_TESS_COMBINE_DATA,
(GLvoid(FONM_GLU_STD_CALLBACK *)()) & CallbackCombine);

Is GLvoid a macro? What's it defined as? It's likely that 19.1.0 got it right by chance, and you need to add something like Macros: [GLvoid(a)=void(*)] to your config file after bcd586b.

[rmarker]

GLvoid isn't a macro. It is a type provided by OpenGL. I.e. via typedef void GLvoid.
Though, adding that macro configuration you suggested, does cause the old formatting to return? Though, I wouldn't think that would be necessary in this instance.
The only macros in that example are FONM_GLU_STD_CALLBACK and GLU_TESS_COMBINE_DATA.

Looking further, I found another example that seems like it could be the same issue. But this one doesn't have any macros present.

19.1.0

Fooooooooooooooooooooo((Type(Baaaaaaaaaaaaaaaaaaar::*)(
    void))&Baaaaaaaaaaaaaaaaaaar::Loooooooooooooooooooooooooooooong);

20

Fooooooooooooooooooooo(
    (Type(Baaaaaaaaaaaaaaaaaaar::*)(void)) &
    Baaaaaaaaaaaaaaaaaaar::Loooooooooooooooooooooooooooooong);

[rmarker]

Appears to have been caused by bcd586b

With the example

Fooooooooooooooooooooo((Type(Baaaaaaaaaaaaaaaaaaar::*)(
    void))&Baaaaaaaaaaaaaaaaaaar::Loooooooooooooooooooooooooooooong);

Before the commit, the & was treated as a unary operator while after it is incorrectly treated as a binary operator.
This being caused by the preceding ) no longer being identified as finishing a cast.

[owenca]

GLvoid isn't a macro. It is a type provided by OpenGL. I.e. via typedef void GLvoid. Though, adding that macro configuration you suggested, does cause the old formatting to return? Though, I wouldn't think that would be necessary in this instance.

You are right because GLvoid is not a macro.

The only macros in that example are FONM_GLU_STD_CALLBACK and GLU_TESS_COMBINE_DATA.

Is FONM_GLU_STD_CALLBACK defined as something like Foo::?

Appears to have been caused by bcd586b

Not really a regression. If fact, it uncovered a bug that the ( after Type below is not annotated. See #126340.

With the example

Fooooooooooooooooooooo((Type(Baaaaaaaaaaaaaaaaaaar::*)(
void))&Baaaaaaaaaaaaaaaaaaar::Loooooooooooooooooooooooooooooong);
Before the commit, the & was treated as a unary operator while after it is incorrectly treated as a binary operator. This being caused by the preceding ) no longer being identified as finishing a cast.

As I mentioned earlier, the ) got annotated only by chance. Anyway, there should be a space between Type and the ( that follows, and the correct format is:

Fooooooooooooooooooooo((Type (Baaaaaaaaaaaaaaaaaaar::*)(
    void))&Baaaaaaaaaaaaaaaaaaar::Loooooooooooooooooooooooooooooong);

[rmarker]

Now that I know more about the inner workings and what to look for, I checked again, and I think I know what is going on now.
Turns out FONM_GLU_STD_CALLBACK is defined as __stdcall.
So I reckon that is the macro that should be defined in the configuration.

Though, I also tried

return (Foo (__stdcall *)(void))&Bar::a;

and that also displays the issue with the left paren not being annotated as a FunctionLParen.
So that appears to be another new bug being identified. I wonder if the annotator needs to check for a tok::kw___stdcall?

Not really a regression. If fact, it uncovered a bug that the ( after Type below is not annotated. See #126340.

I'd argue that from a user perspective the unary operator changing to a binary operator part is a regression, even if it was only getting lucky before ;)
We didn't even notice it was wrong previously with the earlier missing space, but realised something was up when the operator changed.