[RFE] allow assign to members of rvalue aggregate (`-fpermissive`)

[llvmbot]

Bugzilla Link	40670
Version	unspecified
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@DougGregor,@zygoloid

Extended Description

#include <iostream>
#include <cstdlib>
class s {
public:
    int a;
    ~s() { std::cout << "a=" << a << std::endl; }
};
static s foo()
{
    return s{0};
}
int main()
{
    foo().a = 5;
    int *p = std::malloc(sizeof(int));
    return *p;
}

---

This code can be compiled with g++ if -fpermissive is used.
clang++ doesn't compile it with any options.

[llvmbot]

https://lkml.org/lkml/2018/9/13/1272
LKML people also seem to want -fpermissive, and
overall google shows quite a lot of demand.

[llvmbot]

This example demonstrates permissive conversion from
void*, and also an extension to write fields of an aggregate
returned as rvalue. This is a useful extension, because
in the destructor you can have the use of the altered fields,
as the example demonstrates. This is a main motivation why
I think -fpermissive should be supported (permissive conversions
are not so important). g++ supports both, and probably more
extensions under -fpermissive (can't find the exact docs).

[zygoloid]

A general "support -fpermissive" bug is too vague to be actionable -- -fpermissive covers a lot of GNU extensions, some of which we already implement, some of which might be useful but are not currently supported in Clang, and others of which we might want to specifically take a stand against implementing.

Can we repurpose this bug to specifically be about assigning to a subobject of a temporary?

(Incidentally, we should likely also add direct support for a -fpermissive flag, and make it downgrade all of our error-by-default extensions into warnings.)

[llvmbot]

Can we repurpose this bug to specifically be about assigning to a subobject
of a temporary?

Not sure what is the best way of repurpose.
I changed the description.
But will it be activated with -fpermissive, or otherwise?

(Incidentally, we should likely also add direct support for a -fpermissive
flag, and make it downgrade all of our error-by-default extensions into
warnings.)

Would be good.
AFAIK this is all it does.

[llvmbot]

By the way,
https://en.cppreference.com/w/cpp/language/implicit_conversion#Temporary_materialization

The standard seems to say that a temporary materialization
occurs "when performing a member access on a class prvalue;",
which seems to be the case here.
So I even wonder why it is not assignable by default?
Is materialization not sufficient to became assignable?

[zygoloid]

Temporary materialization conversion converts the s prvalue to an s xvalue. Class member access then gives an int xvalue. Assignment needs an lvalue; an xvalue won't do.

[stsp]

Hi, what would be a resolution of this?
Since clang cares that much with gcc
compatibility recently, would it be possible
to implement that gcc extension in clang?
If -fpermissive is a bad name, then some
-fassignable-xvalue can be used, or alike.

[shafik]

This request does not seem particularly strongly motivated at least as explained currently. This case can covered using designated initializers e.g. s{.a=10}, full example: https://godbolt.org/z/Ea78vG3Ex

#include <iostream>
#include <cstdlib>
class s {
public:
    int a;
    ~s() { std::cout << "a=" << a << std::endl; }
};
static s foo()
{
    return s{0};
}
int main()
{
    (void)s{.a=10};
    int *p = (int*)std::malloc(sizeof(int));
    return *p;
}

If we do a have a more strongly motivated case, I believe a RFC on Discourse would be more appropriate today.

CC @AaronBallman

[stsp]

If it would be a new code then maybe
a different syntax would be possible.
But currently I need to deal with a huge
code that does foo().a = 5; as shown
in my example.
Its just that currently foo is defined
this way:

wrp_type& foo() { return *new(get_buf()) wrp_type; }

which means the destructor is never called
implicitly. I wanted to change the definition
of foo this way:

wrp_type foo() { return wrp_type(get_buf()); }

in which case after
foo().a = 5; the destructor would be called.
Unfortunately that works only on gcc.

Basically speaking, there are many syntaxes
that can be replaced with something else in
theory. But in practice you are not going to
rewrite the entire code base or change the
API you already provided your users with,
so you want some particular syntax even if
the problem can be solved differently.

[AaronBallman]

We've traditionally been strongly opposed to -fpermissive in Clang, and I don't think that position has changed. This is effectively asking for part of -fpermissive's functionality, which we've considered in the past, but generally only when the code shows up in some popular third-party library or other scenario where changing the code is effectively impossible. It doesn't sound to me like that's the case here. So I don't think this is a motivated change, unless I'm missing some information.

[stsp]

I don't think my code represent a popular
third-party library (only about ~50-100 clones
in 2 weeks, per github stats), and I don't think
"changing the code is effectively impossible"
is my case as well because currently my
code is clang-specific anyway. Its just that
the recent clang-16 changes scared me a
lot so I started to think about the gcc porting.
gcc porting will require that functionality and
I don't want to became clang-incompatible
in a process.

Anyway, I don't propose to judge my code.
I propose to evaluate the extension itself.
To me, this looks like a tiny extension that
only requires to remove some check that
prevents the assignment to temporary from
happening. If that extension is really as
small as I think of it, and if both compilers
support it, then what prevents it from becoming
the part of the standard? :)
But if you think this is a difficult to implement
extension or is completely useless, then of
course its another story.

This is effectively asking for part of -fpermissive's functionality,
which we've considered in the past

Please provide me with an URL of such a
discussion, if possible.

[AaronBallman]

But if you think this is a difficult to implement
extension or is completely useless, then of
course its another story.

"Completely useless" is a bit heavier of a term than I'd use, but it's still pretty unmotivated (at least, to me). One of the criteria for adding extensions to Clang (https://clang.llvm.org/get_involved.html#criteria) is that there be evidence of a significant user community. I think that's the primary thing that I'm missing which would motivate me.

Please provide me with an URL of such a
discussion, if possible.

Oddly, this came up literally this morning: #54120 -- but there are other mentions of this on the mailing lists and bug database. Also: https://reviews.llvm.org/D58154

[stsp]

Thanks.
Those are about an -fpermissive itself.
If most of its extensions are ugly and
require AST changes, then of course
they shouldn't be added.

This particular extension wasn't discussed
though. To me it looks very small and
natural, I am not even sure why it was
restricted by the standard to not work
that way. So it would be good to at least
find out your opinion first on whether
its "ugly" or not, as you marked another
extensions that way. If you think its among
them, then why to even bother. I only
want a gcc port of my code, so I want to
use the "good" extensions. :)

[AaronBallman]

This particular extension wasn't discussed though.

Correct, I've not seen a discussion of this particular aspect aside from on this thread.

To me it looks very small and natural, I am not even sure why it was restricted by the standard to not work that way.

See: #40016 (comment); the value category is important in terms of correctness, move semantics, and optimization opportunities.

So it would be good to at least find out your opinion first on whether its "ugly" or not, as you marked another extensions that way. If you think its among them, then why to even bother. I only want a gcc port of my code, so I want to use the "good" extensions. :)

I don't think it's a good extension to add. There are very, very few circumstances under which allowing assignment into an xvalue is reasonable, so adding this extension increases the chances of running into more bugs in order to work around what amounts to a design issue. You've found one of the cases where you could make an argument that the assignment is reasonable, but I think this is a quite rare situation, so it's not that compelling. Essentially, I don't think this meets the bar for an extension we'd be interested in adding to Clang, at least without further evidence that there is a greater need.

(To be clear, this is not "no, we'll never do it" to this particular request -- this is "no, we need more evidence that there's sufficient community need for a new language dialect".)