Bug 84667 - unreasonable refusal to use assignment operator method
Summary: unreasonable refusal to use assignment operator method
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 6.3.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-02 10:13 UTC by Elmar Stellnberger
Modified: 2018-03-05 11:22 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-03-02 00:00:00


Attachments
sample program that evokes the given error (39.90 KB, application/x-tar)
2018-03-02 10:13 UTC, Elmar Stellnberger
Details
reduced sample program that evokes the given error (8.56 KB, application/x-tar)
2018-03-02 13:15 UTC, Elmar Stellnberger
Details
fixed version of auxtypes.h which works under g++ and clang (4.94 KB, text/plain)
2018-03-04 10:40 UTC, Elmar Stellnberger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Elmar Stellnberger 2018-03-02 10:13:28 UTC
Created attachment 43542 [details]
sample program that evokes the given error

g++ (Debian 6.3.0-18+deb9u1) 6.3.0 20170516:

At me g++ refuses to make use of the following operator
  xstrbuf& operator = ( base_str_const s );

Instead it seems to convert an xstr_const into an xstr_mutable which is generally forbidden and not enabled by my code.

Things work well however if I use the standard constructor instead of an assignment:

  inline xstrbuf( base_str_const s ) : base_str( const_cast<char_type*>(s.chars), s.length ) { bufParams = xstrbuf_constBuf; };  

It is important to get the "bufParams = xstrbuf_constBuf; " executed otherwise the code will fail. You can test for it yourself by executing ./runit.

  Any feedback would be very appreciated!
Comment 1 Elmar Stellnberger 2018-03-02 13:15:19 UTC
Created attachment 43549 [details]
reduced sample program that evokes the given error

sample program that evokes the error reduced to 20% of its original size
Comment 2 Elmar Stellnberger 2018-03-02 14:16:17 UTC
Princess17b29a just found out that the problem can be resolved by adding the const keyword to the constructor in line 233:
inline xstrbuf( const xstrbuf& s ) ...
... as neither "xstrbuf( base_str_const s )" nor "xstrbuf& operator = ( base_str_const s )" are used directly. An error message of clang has hinted us to do so:

test_it.cpp:11:11: error: no viable constructor copying variable of type 'estrbuf' (aka 'xstrbuf<short>')
  estrbuf copybuf1 = varbuf4.as_const(); // copybuf.bufParams = xstrbuf_constBuf;
          ^          ~~~~~~~~~~~~~~~~~~
./auxtypes.h:233:10: note: candidate constructor not viable: expects an l-value for 1st argument
  inline xstrbuf( xstrbuf& s ) : base_str( s ) { bufParams = s.bufParams | xstrbuf_bufUserAllocatedBit; };      // new xstrbuf then holds a temporary copy of the initial xstrbuf
         ^
1 error generated.
Comment 3 Elmar Stellnberger 2018-03-02 14:38:44 UTC
if I unify the implicit copy constructor with my own one then it also works for g++:   xstr_mutable( const xstr& s ) : xstr(s) {}
Comment 4 David Malcolm 2018-03-02 15:27:25 UTC
FWIW, I'm also seeing:

auxtypes.cpp: In member function ‘aux::xstrbuf<xchar>& aux::xstrbuf<xchar>::operator=(const char*) [with xchar = short int]’:
auxtypes.cpp:240:3: warning: ‘blen’ is used uninitialized in this function [-Wuninitialized]
   assignByLength( blen, s );   // side effect input; base_str::length
   ^~~~~~~~~~~~~~
auxtypes.cpp: In member function ‘aux::xstrbuf<xchar>& aux::xstrbuf<xchar>::operator=(const char*) [with xchar = int]’:
auxtypes.cpp:240:3: warning: ‘blen’ is used uninitialized in this function [-Wuninitialized]
   assignByLength( blen, s );   // side effect input; base_str::length
   ^~~~~~~~~~~~~~
auxtypes.cpp: In member function ‘aux::xstrbuf<xchar>& aux::xstrbuf<xchar>::operator=(const char*) [with xchar = signed char]’:
auxtypes.cpp:240:3: warning: ‘blen’ is used uninitialized in this function [-Wuninitialized]
   assignByLength( blen, s );   // side effect input; base_str::length
   ^~~~~~~~~~~~~~

utf8len presumably is meant to write back to &blen in the line above, but the decl and defn of the:
  strsize utf8len( const char *buf, strsize *byte_length = NULL );
overload are labelled "pure".  Removing the "pure" fixes that warning.
Comment 5 Jonathan Wakely 2018-03-02 17:27:03 UTC
(In reply to Elmar Stellnberger from comment #1)
> sample program that evokes the error reduced to 20% of its original size

At least 90% of it is still completely irrelevant and we have to wade through hundreds of lines of macros and unrelated junk. What exactly is the bug?

If you simply need to add const to your copy constructors and copy assignment operators, well that's how C++ works.
Comment 6 Jonathan Wakely 2018-03-02 17:31:47 UTC
(In reply to Elmar Stellnberger from comment #0)
> Things work well however if I use the standard constructor instead of an
> assignment:
> 
>   inline xstrbuf( base_str_const s ) : base_str(
> const_cast<char_type*>(s.chars), s.length ) { bufParams = xstrbuf_constBuf;
> };  

Instead of what? Where are we supposed to be looking?

>   Any feedback would be very appreciated!

Your code is almost unreadable.
Comment 7 Jonathan Wakely 2018-03-03 00:16:23 UTC
OK I've finished debugging this horribly obfuscated program, and this is not a compiler bug.

This is copy-initialization:

  estrbuf copybuf1 = varbuf4.as_const();

That means it's equivalent to this:

  estrbuf copybuf1 = estrbuf(varbuf4.as_const());

This constructs a temporary strbuf which has the expected value (0x80000000) but then tries to copy-construct another estrbuf. Your copy constructor cannot be used to copy temporaries, because you declared it so it only works for lvalues:

  inline xstrbuf( xstrbuf& s )

so instead the temporary gets converted to its base class (via slicing) and calls this constructor:

  inline xstrbuf( base_str s )

and that constructor sets bufParams to:

     bufParams = s.length | xstrbuf_bufUserAllocatedBit;

Which explains the unwanted 0xb in the value 8000000b, it comes from s.length

In the second case you use direct-initialization:

  estrbuf copybuf2( varbuf4.as_const() );

so there is no temporary, no slicing, and no copy that gets s.length in the params.


You should fix your copy constructor to accept const-lvalues, and avoid slicing, (and stop misusing the pure attribute, and remove the useless register keywords, and use more whitespace to help readability, and stop using so many macros, ...)
Comment 8 Elmar Stellnberger 2018-03-04 10:30:24 UTC
concerning comment #4:  Why do I not get the warning of utf8len being used uninitialized? I have compiled with -Wall and -Wmaybe-uninitialized. This would not be the first time that g++ does not notify me about a variable that I have forgotten to initialize. I have already wasted a lot of time in not knowing that a certain variable was not initialized in another context.
Comment 9 Elmar Stellnberger 2018-03-04 10:37:39 UTC
I still do not understand why this constructor gets called: inline xstrbuf( base_str s ). If I use .as_const() the result should be an xstr_const : public xstr_abstract<const xchar,xchar> and not an xstr_mutable: public xstr_abstract<xchar,xchar>. Even if it ascended to xstr_abstract it would have to discard the const qualifyer.
Comment 10 Elmar Stellnberger 2018-03-04 10:40:58 UTC
Created attachment 43557 [details]
fixed version of auxtypes.h which works under g++ and clang

Well if I fix things for clang then the code will also do the right thing under g++. However this does not resolve my initial question.
Comment 11 Elmar Stellnberger 2018-03-04 19:09:59 UTC
  Why does my gcc not report uninitialized variables. Is the version too old (6.3.0 20170516)? 
  Still I do not know how g++ can convert an xstr_const into an xstr_mutable. That should be impossible as stated in the comment above. Thus the fact that correcting an error reported by clang has made the program behave correctly under g++ is simply astonishing. However I have no idea why g++ behaves like this.
Comment 12 Jonathan Wakely 2018-03-05 11:22:29 UTC
(In reply to Elmar Stellnberger from comment #9)
> I still do not understand why this constructor gets called: inline xstrbuf(
> base_str s ). If I use .as_const() the result should be an xstr_const :
> public xstr_abstract<const xchar,xchar> and not an xstr_mutable: public
> xstr_abstract<xchar,xchar>. Even if it ascended to xstr_abstract it would
> have to discard the const qualifyer.

Read my answer again more carefully.

> This is copy-initialization:
>
>  estrbuf copybuf1 = varbuf4.as_const();
>
> That means it's equivalent to this:
>
>  estrbuf copybuf1 = estrbuf(varbuf4.as_const());

The result of as_const() is an xstr_const, but then a temporary xstrbuf is created from that, and then because your copy constructor was not viable for copying a temporary (it only accepts non-const lvalues) the temporary is converted to its base class (xstr_mutable) and then the xstrbuf(base_str) constructor is used.

If you write complicated, unreadblae code then don't be surprised if you can't understand what it does!


(In reply to Elmar Stellnberger from comment #11)
>   Why does my gcc not report uninitialized variables. Is the version too old
> (6.3.0 20170516)? 

Yes, I see warnings with GCC 7.1 and later.

>   Still I do not know how g++ can convert an xstr_const into an
> xstr_mutable. That should be impossible as stated in the comment above. Thus
> the fact that correcting an error reported by clang has made the program
> behave correctly under g++ is simply astonishing. However I have no idea why
> g++ behaves like this.

Because that's what the C++ standard says should happen.

I don't get any error compiling yuor code with clang, I see exactly the same behaviour as GCC.