Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang++ incorrectly accepts addition with an array prvalue operand #54016

Open
languagelawyer opened this issue Feb 23, 2022 · 14 comments
Open
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@languagelawyer
Copy link

languagelawyer commented Feb 23, 2022

int main()
{
    using IA = int[];

    IA{ 1, 2, 3 } + 0;
}

[expr.add]/1:

For addition, either both operands shall have arithmetic or unscoped enumeration type, or one operand shall be a pointer to a completely-defined object type and the other shall have integral or unscoped enumeration type.

Array types are not allowed. Neither there is wording requiring the array-to-pointer conversion on a prvalue array operand.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Feb 23, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 23, 2022

@llvm/issue-subscribers-clang-frontend

@languagelawyer languagelawyer changed the title clang: incorrectly accepts addition with an array prvalue operand clang++ incorrectly accepts addition with an array prvalue operand Feb 23, 2022
@shafik
Copy link
Collaborator

shafik commented Feb 28, 2022

I believe clang is correct here, we apply a standard conversion [conv.general] which includes array-to-pointer conversion [conv.array] which results in a temporary materialization conversion [conv.rval].

A quick godbolt with AST output seems to confirm this what clang is doing.

This 2018 cfe-dev mailing on a related case seems to indicate that clang is correct and although "GCC's behaviour is not unreasonable" CC @zygoloid

Clang has -Waddress-of-temporary but looking at the test suite in:

  clang/test/SemaCXX/address-of-temporary.cpp

It looks like we purposely do not consider the array-to-pointer conversion to be included as taking the address of a temporary.

Although I can see the case can be made for clang to be stricter.

Note, -Wdangling will catch the case where we pointer to the temporary outlives the temporary.

@languagelawyer
Copy link
Author

I believe clang is correct here, we apply a standard conversion [conv.general]

And why it should be applied here?

@zygoloid
Copy link
Collaborator

I believe clang is correct here, we apply a standard conversion [conv.general]

And why it should be applied here?

I see, your argument is that [basic.lval]/6 does not apply because the array operand is not a glvalue? I don't think that's the intent of CWG1232, but it does seem to be what the current wording says. This seems worth a CWG discussion before we proceed.

@languagelawyer
Copy link
Author

your argument is that [basic.lval]/6 does not apply because the array operand is not a glvalue?

Exactly.

This seems worth a CWG discussion

e-mail thread or immediate issue request?

@zygoloid
Copy link
Collaborator

zygoloid commented Mar 5, 2022

I think it's OK to go straight to filing an issue (github.com/cplusplus/cwg/issues), given that I think it's at least unclear whether the wording reflects the intent.

@shafik
Copy link
Collaborator

shafik commented Mar 5, 2022

Also would like to point out [conv.general]p2.1 says:

Expressions with a given type will be implicitly converted to other types in several contexts:

  • When used as operands of operators.
    The operator's requirements for its operands dictate the destination type ([expr.compound]).

Although this is a note and this non-normative I think it speaks to the intent.

@jensmaurer
Copy link

Created core issue 2548.

@languagelawyer
Copy link
Author

  • The operator's requirements for its operands dictate the destination type ([expr.compound]).

Although this is a note and this non-normative I think it speaks to the intent.

No, it is just a defective Note. See https://timsong-cpp.github.io/cppwp/n4868/dcl.init.general#def:destination_type. Operand of a built-in operator is not «object or reference being initialized».

@pinskia
Copy link

pinskia commented Oct 9, 2023

Note the GCC issue side is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94264 .

@zygoloid
Copy link
Collaborator

zygoloid commented Oct 9, 2023

CWG forwarded the issue to EWG: cplusplus/papers#1633

There's no clear indication yet of whether prvalue arrays should be subject to the array-to-pointer conversion when they appear as the operand of [] or unary *.

@languagelawyer
Copy link
Author

when they appear as the operand of [] or unary *.

You mean + instead of []?

@Fedr
Copy link

Fedr commented Oct 9, 2023

Related discussion: https://stackoverflow.com/q/77051011/7325599

@zygoloid
Copy link
Collaborator

zygoloid commented Oct 9, 2023

You mean + instead of []?

I meant [], as I wrote, but unary + is another operator for which the same concerns apply. (I mentioned this to Jens yesterday and the CWG issue has already been updated to cover that case.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

8 participants