Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Missing implicit conversion between variably modified pointers in C++ #34854

Open Quuxplusone opened 6 years ago

Quuxplusone commented 6 years ago
Bugzilla Link PR35881
Status NEW
Importance P enhancement
Reported by Ignat Loskutov (ignat.loskutov@gmail.com)
Reported on 2018-01-09 15:10:37 -0800
Last modified on 2018-01-09 23:25:21 -0800
Version trunk
Hardware PC All
CC dgregor@apple.com, efriedma@quicinc.com, hfinkel@anl.gov, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
Consider the following example:

int foo(int *ptr, size_t rows, size_t cols) {
    int (*m)[cols] = (int(*)[cols])(ptr);
    return m[rows - 1][cols - 1];
}

It fails to compile in C++ mode with a weird error message:

1 : <source>:1:11: error: cannot initialize a variable of type 'int (*)[cols]'
with an rvalue of type 'int (*)[cols]'
    int (*ptr)[cols] = (int(*)[cols])(m);
          ^            ~~~~~~~~~~~~~~~~~

Although VLAs are not a part of C++ standard, Clang claims to support them for
compatibility with GCC. GCC, ICC, and Clang in C mode compile this piece of
code successfully.
Quuxplusone commented 6 years ago
The problem is that the two types aren't the same, so the compiler needs a
special rule to allow this implicit conversion (and the relevant code isn't
shared between C and C++).

You can work around the issue using a typedef ("typedef int COLS[cols]; COLS*m
= (COLS*)(ptr);").
Quuxplusone commented 6 years ago
Consider:

    int (*m)[cols] = (int(*)[rows])(ptr);

This is valid C, but we (and GCC) reject it in C++ mode. I consider this a
feature; C++ has stronger type checking on assignment, and generally tries to
only allow implicit conversions in pointer assignment if they are sound.

Now consider:

    int (*m)[cols];
    cols = rows;
    m = (int(*)[cols])(ptr);

GCC now accepts this in C++, but Clang rejects. I still consider this to be a
Clang feature, for the same reason. The assignment is still unsound.

Finally, the original code:

    int (*m)[cols] = (int(*)[cols])(ptr);

... is only correct because 'cols' didn't change between the two uses to form
two variable length array types. And it's not reasonable to expect the compiler
to check whether the array bound has changed between the two expressions when
determining whether the right hand side can be converted to the type of the
left hand side.

So, while the example in comment#0 is a false positive, I think that's in line
with C++ only allowing implicit pointer conversions that are trivially sound,
and requiring explicit casts in the remaining cases. (In this case, the cast
can be performed with either a typedef or a construct like
"(decltype(m))(ptr)").
Quuxplusone commented 6 years ago

I think it would make sense to expect the variable hasn't changed at least if it has a const qualifier.