[Development] QUIP 6: removing top-level const from return types

[Andre Poenitz]

marc.mutz wrote:
> On Tuesday 23 May 2017 00:48:53 Thiago Macieira wrote:
> > Then we are right now concluding this kind of change should not be in that
> > part of the QUIP.
>
> The QUIP gives an _algorithm_ to categorise SiCs, it's not a listing. There's
> a list, yes. It's called _examples_.
>
> > if there's a chance of existing code breaking
>
> How can removing top-level const from a return type "break" existing code any
> more than adding a function overload?

"Any more" is an interesting, but unrelated question.

In general, both kinds of changes are able to introduce regressions 
in code size and cycles spent, and therefore should not be done 
carelessly.

Compare

#include <QVector>

const QVector<int> constFoo();

int useConstFoo()
{
    return constFoo()[0];
}

_Z11useConstFoov:
        pushq   %rbx
        subq    $16, %rsp
        movq    %rsp, %rdi
        call    _Z8constFoov at PLT
        movq    (%rsp), %rdi
        movq    16(%rdi), %rax
        movl    (%rdi,%rax), %ebx
        movl    (%rdi), %edx
        testl   %edx, %edx
        je      .L6
        cmpl    $-1, %edx
        je      .L3
        lock subl       $1, (%rdi)
        je      .L9
.L3:
        addq    $16, %rsp
        movl    %ebx, %eax
        popq    %rbx
        ret
.L9:
        movq    (%rsp), %rdi
.L6:
        movl    $8, %edx
        movl    $4, %esi
        call    _ZN10QArrayData10deallocateEPS_mm at PLT
        addq    $16, %rsp
        movl    %ebx, %eax
        popq    %rbx
        ret

-- vs --


QVector<int> nonConstFoo();

int useNonConstFoo()
{
    return nonConstFoo()[0];
}


_Z14useNonConstFoov:
        pushq   %rbx
        subq    $16, %rsp
        movq    %rsp, %rdi
        call    _Z11nonConstFoov at PLT
        movq    (%rsp), %rax
        movl    (%rax), %edx
        cmpl    $1, %edx
        jbe     .L31
        movl    8(%rax), %edx
        andl    $2147483647, %edx
        je      .L32
        movl    4(%rax), %esi
        xorl    %ecx, %ecx
        movq    %rsp, %rdi
        call    _ZN7QVectorIiE11reallocDataEii6QFlagsIN10QArrayData16AllocationOptionEE at PLT
        movq    (%rsp), %rdx
.L23:
        movq    16(%rdx), %rax
        movl    (%rdx,%rax), %ebx
        movl    (%rdx), %ecx
        testl   %ecx, %ecx
        je      .L29
.L25:
        cmpl    $-1, %ecx
        je      .L26
        lock subl       $1, (%rdx)
        je      .L29
.L26:
        addq    $16, %rsp
        movl    %ebx, %eax
        popq    %rbx
        ret
.L32:
        xorl    %edx, %edx
        movl    $2, %ecx
        movl    $8, %esi
        movl    $4, %edi
        call    _ZN10QArrayData8allocateEmmm6QFlagsINS_16AllocationOptionEE at PLT
        movq    %rax, %rdx
        movq    %rax, (%rsp)
        movq    16(%rdx), %rax
        movl    (%rdx,%rax), %ebx
        movl    (%rdx), %ecx
        testl   %ecx, %ecx
        jne     .L25
.L29:
        movq    (%rsp), %rdi
        movl    $8, %edx
        movl    $4, %esi
        call    _ZN10QArrayData10deallocateEPS_mm at PLT
        addq    $16, %rsp
        movl    %ebx, %eax
        popq    %rbx
        ret
.L31:
        movq    %rax, %rdx
        jmp     .L23


I hope you agree that the version with removed top-level const
is worse in this case.


But again, the reason for refusing the QSplashScreen::pixmap() 
change was the ugliness of the code and the introduction of a 
compiler dependency. The patch does not even what it claims 
to do, namely to fix the supposed API problem in Qt 5.

The proper solution here would have been to do the change for 
Qt 6. Neither the macro ugliness would be needed in this case
nor would we have a compiler dependency.

If you insist that such a change is allowed by QUIP-6 (and actually
I agree that it is by the current wording) then QUIP-6 should be 
amended to disallow such undesirable changes.

> Thanks,
> Marc

You are welcome,
Andre'