llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.11k stars 12.01k forks source link

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

Open languagelawyer opened 2 years ago

languagelawyer commented 2 years ago
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.

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-frontend

shafik commented 2 years ago

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 commented 2 years ago

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

And why it should be applied here?

zygoloid commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Created core issue 2548.

languagelawyer commented 1 year ago
  • 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 commented 1 year ago

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

zygoloid commented 1 year ago

CWG forwarded the issue to EWG: https://github.com/cplusplus/papers/issues/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 commented 1 year ago

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

You mean + instead of []?

Fedr commented 1 year ago

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

zygoloid commented 1 year ago

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