Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

[SOLVED] apparent c++ compiler issue compiling a throw statement #32448

Open Quuxplusone opened 7 years ago

Quuxplusone commented 7 years ago
Bugzilla Link PR33476
Status NEW
Importance P normal
Reported by Tim (tootall79@gmail.com)
Reported on 2017-06-15 19:35:54 -0700
Last modified on 2017-06-20 10:28:28 -0700
Version 3.8
Hardware PC FreeBSD
CC anton@korobeynikov.info, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
while compiling a try block compiler cannot find the correct ctor candidate,
yet it CAN find it when compiling outside the throw line.

here are 3 lines of code, the first 2 lines compiles fine, the 3rd line gives
the error shown

                TString sBS("hello world");
        TErrPub eFU( sBS );           // no error here
        throw TErrPub( sBS );         // error here ??

the first line demonstrates the sBS, a string object, DOES have a ctor in the
TErrPub class, and the compiler finds it fine.  But if you try to construct the
exact same TErrPub instance inside a throw it can't find the ctor.  Here is the
error:

/media/FATSD29/Houston/wkspc/free/dev/wxhSLED/main.cpp:740:9: error: no
matching constructor for initialization of 'TErrPub'
                throw TErrPub( sBS );
                      ^~~~~~~~~~~~~~
../../../../include/TErrPub.h:15:2: note: candidate constructor not viable:
expects an l-value for 1st argument
        TErrPub( TErrPub& EP );
        ^
../../../../include/TErrPub.h:17:2: note: candidate constructor not viable: no
known conversion from 'TErrPub' to 'TString &' for 1st argument
        TErrPub( TString& sMsg );

please advise, thanks!
Quuxplusone commented 7 years ago

Please attach the full preprocessed source for the code in question

Quuxplusone commented 7 years ago

Throwing an exception requires the exception object to be copyable: http://eel.is/c++draft/except.throw#5

We should extend our diagnostic in this case to explain what construction we're performing here, since it apparently does not currently say that we're trying to copy the exception object.

Quuxplusone commented 7 years ago
Thanks for your attention to this!

Anton
I have the preprocessed file, it is very large over 4MB mostly irrelevant, do
you want top see the whole thing or some subset?  It is too large to attach but
I can paste it into a comment.

Richard
Do I need to override = ?
Its hard to say where the error is, whether its the first ctor of TErrPub or if
its in the copy ctor.  But both ctors exist!  I thought copy meant constructing
from a reference of self.

class A
    A( A& CopyMe );    // copy constructor

The diagnostics are excellent, it goes through all the available ctors and
explains why they are not valid.  to make a copy DOES require a ctor, but the
matching ctor IS there, and it is rejected.  the copy ctor is shown AND the one
used for the original object constructed from TString.

in other words, the object CAN be copied AND it can be constructed from TString

/media/FATSD29/Houston/wkspc/free/dev/wxhSLED/main.cpp:740:9: error: no
matching constructor for initialization of 'TErrPub'
                throw TErrPub( sBS );
                      ^~~~~~~~~~~~~~
../../../../include/TErrPub.h:15:2: note: candidate constructor not viable:
expects an l-value for 1st argument
        TErrPub( TErrPub& EP );
        ^
../../../../include/TErrPub.h:17:2: note: candidate constructor not viable: no
known conversion from 'TErrPub' to 'TString &' for 1st argument
        TErrPub( TString& sMsg );
Quuxplusone commented 7 years ago

I added operator = to class TErrPub. no help.

Quuxplusone commented 7 years ago
Here is a subset of the large preprocessed file, the operator = was added and
tested AFTER this file was generated:

# 1 "/media/FATSD29/Houston/include/TStrPub.h" 1
# 11 "/media/FATSD29/Houston/include/TStrPub.h"
#pragma message("TStrPub__INCLUDED_")
# 42 "/media/FATSD29/Houston/include/TStrPub.h"
 class TString {
 public:
  TString();

  TString( TString& s );
  TString( wxString& s );
  TString( const char * psz );
  TString( const wchar_t * wcP );

  static TString msNull;
  static TString ltoa( long lSrc, int iZpad = 0, int iBase = 0 );
# 74 "/media/FATSD29/Houston/include/TStrPub.h"
  static TString FormatV(const wxString& format, va_list argptr);
  int PrintfV(const wxString& format, va_list argptr);

  LPTSTR GetBuffer( int nMinBufLength );
  void ReleaseBuffer( int iCount );
  ulong GetLength();

  operator wxString*();

  TString& Format( char const * pszFormat, ...);

  TString& FF( LPCTSTR pszFormat, ...);
  TString& FF( wchar_t * pszFormat, ...);
  void * GetHS();
  bool IsEmpty();

  TString& operator=( TString& sSrc );
  TString& operator=( wxString& sSrc );

  TString& operator=( char const * cpPath );
# 118 "/media/FATSD29/Houston/include/TStrPub.h"
  TString operator+=( LPCTSTR lpszPath );
  TString operator+=( TString& sSrc );
  bool operator==( TString& sSrc );
  bool operator==( LPCTSTR lpszSrc );
  bool operator<( TString& sSrc );
  bool operator>( TString& sSrc );
  bool operator<=( TString& sSrc );
  bool operator>=( TString& sSrc );

  LPTSTR GetBufConst();
  LPTSTR GetBufferSetLength( int nNewLength );
  int Replace( LPCTSTR lpszOld, LPCTSTR lpszNew, BOOL bReplaceAll = 1 );
  operator LPCTSTR();

  TString Left( size_t nCount );
  TString Right( size_t nCount );
  TString Mid( size_t nFirst, size_t nCount );
  TString Mid( size_t nFirst );

  size_t GetLengthPlusNullInBytes();
  int GetType();
  int Find( TString& sSub, int iStartPos = 0 );
  int Find( LPCTSTR pSub, int iStartPos = 0 );
  int ReverseFind( TCHAR ch );
  int Compare(LPCTSTR lpsz);
  int CompareNoCase(LPCTSTR lpsz);

  void Empty();
  void MakeLower();
  void MakeUpper();
  void Alloc( size_t iSize );

    protected:
  void * GetStringData();
  void __attribute__((__fastcall__)) FreeData( void * vpData );

  void * mvpBuf;

    };
# 270 "/media/FATSD29/Houston/include/TStrPub.h"
typedef TString QQ;

# 8 "/media/FATSD29/Houston/include/TErrPub.h" 2

class TErrPub
{
public:

 TErrPub();
 TErrPub( TErrPub& EP );

 TErrPub( TString& sMsg );

 TErrPub( TErrPub * e );
 TErrPub( TErrPub * e, STPub& STP );
 TErrPub( TString& sMsg, STPub& STP );
 TErrPub( long& lErrNo, STPub& STP );
 TErrPub( STPub& STP );

 TErrPub( LPCTSTR npLocName );
 TErrPub( LPCTSTR cpLocName, STPub& STP );
 TErrPub( int iErrNo );

 TErrPub( void * pPS, int iFId, int iFF, LPCTSTR lpszFormat, ... );

 virtual long GetVersion();

 static TString GetMSEStr( TErrPub * e, LPCTSTR pBuf );

 virtual void Caught( LPCTSTR pWhere );

 virtual void Caught( TString& sBuf );

 TString msThrowMsg;
};

bool MyApp::OnInit()
{
    if ( !wxApp::OnInit() )
        return false;
# 600 "main.cpp"
 int iWcharSize = sizeof(wchar_t);
 std::string ssss("hello stand string");
 std::wstring ssws(L"hello stand wstring");

 char16_t saHello[6];
 saHello[0] = 'H';
 saHello[1] = 'e';
 saHello[2] = 'l';
 saHello[3] = 'l';
 saHello[4] = 'o';
 saHello[5] = 0;

 char32_t saHello32[6];
 saHello32[0] = 'H';
 saHello32[1] = 'e';
 saHello32[2] = 'l';
 saHello32[3] = 'l';
 saHello32[4] = 'o';
 saHello32[5] = 0;

 std::u16string ssu16( saHello, 6 );
 const char16_t * pZ = ssu16.c_str();
 std::u16string ssu16b( saHello );

 std::u32string ssu32( saHello32, 6 );
 std::u32string ssu32b( saHello32 );
 const char32_t * pZ2 = ssu32b.c_str();

 TString ssTryThis;
 TString ssS("yes"), ssRes, sUnicode;

  sUnicode.Format("wxUSE_UNICODE is true");

 ssRes = ssTryThis.Format( "Hello %d World %s sizeof(whcar_t) %d", 996, &ssS, iWcharSize );
 wxString* pX = (wxString*) ssRes.GetHS();
 pX = (wxString*) sUnicode.GetHS();

 TLogFilePub LFP;
 gpLogFile = LFP.MakeLFPub();

  TString sPref(L"SM"), sHostName(L"170412 FBSD HelloWorld");

 wxString* pS = (wxString*) sPref.GetHS();
 gpLogFile->Init( sPref );
 gpLogFile->SetVPVal( this, evpMiscVPId01 );
 long lTicks = GetTickCount();

 Hack((void *) lTicks, 5, "lT First Hack *.a %s %d", &sHostName, lTicks );
 TString sEther;
 TDevSig DS;
 DS.GetDevSig( sEther );
 TDevSig gds;
 uchar * pMAC = 0;
 long len = gds.GetMACAddressFreeBSD( pMAC );
 TString sMAC( (char *) pMAC );
 Hack((void *) lTicks, 5, "lT GotMAC %s %d", &sMAC, lTicks );
 Hack((void *) lTicks, 5, "lT GotMAC %s %s %d", &sEther, &sMAC, lTicks );

 TString sFrag("yolo county");
 TString sShouldWork( TString().FF("copy ctor and macros %d %s", 123, &sFrag ) );
 Hack((void *) lTicks, 5, "170613 should work: %s", &sShouldWork );
# 697 "main.cpp"
    wxFrame* frame = new MyFrame(__null,
                                 wxID_ANY,
                                 L"wxAUI Sample Application",
                                 wxDefaultPosition,
                                 wxSize(800, 600));

    frame->Show();

 try {
  TString sHello("Hello");

  TString sBS;

  sBS.FF(("this 0x%x QQFF string sHello %s"), this, &sHello );
# 739 "main.cpp"
  TErrPub eFU( sBS );
  throw TErrPub( sBS );

    } catch ( TErrPub& e) {

  TString sHello("Goodbye");
  e.Caught( sHello );
    }
    return true;
}
Quuxplusone commented 7 years ago
Same bug, different program, full source is included below, here is the
diagnostic:

why is the compiler trying to convert 1st argument??
no known conversion from 'TErr' to 'TStr &' for 1st argument

/usr/bin/clang++  -c "/media/FATSD29/Houston/wkspc/free/templ/simpcon/main.cpp"
-O2 -Wall -DNDEBUG -o./Release/main.cpp.o -I. -I.
/media/FATSD29/Houston/wkspc/free/templ/simpcon/main.cpp:53:9: error: no
matching constructor for initialization of 'TErr'
                throw TErr( TStr().Format("PANIC NOT! Hello World %d", 99) );
                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/media/FATSD29/Houston/wkspc/free/templ/simpcon/main.cpp:43:2: note: candidate
constructor not viable: expects an l-value for 1st argument
        TErr( TErr& EP ) { msMsg = EP.msMsg; }
        ^
/media/FATSD29/Houston/wkspc/free/templ/simpcon/main.cpp:44:2: note: candidate
constructor not viable: no known conversion from 'TErr' to 'TStr &' for 1st
argument
        TErr( TStr& sMsg ) { msMsg = sMsg; }
        ^
/media/FATSD29/Houston/wkspc/free/templ/simpcon/main.cpp:42:2: note: candidate
constructor not viable: requires 0 arguments, but 1 was provided
        TErr() { ; }
        ^
1 error generated.

preprocessed code is 1.2 MB but almost all of it is from one include file:
#include <iostream>

I am guessing you don't need that file, its a standard file unedited by us.
the actual program is 60 lines of code, which is included below:

#include <iostream>

class TStr {                // LPCTSTR
public:
    TStr()  { ; }
    TStr( const char * cp ) : mss(cp) { ; }     // string literal
    TStr( TStr& s ) { mss = s.mss; }                            // copy ctor

    TStr&   operator=( TStr& sSrc ) { mss = sSrc.mss; return *this; }       // you only
need one of these 161028
//  TStr& Format( char * cpFmt, ...);
    TStr& Format( const char * cpFmt, ...);
    std::string mss;
};

//  vsnprintf
//  int vsnprintf (char * s, size_t n, const char * format, va_list arg );
class TStr;
TStr& TStr::Format( const char * lpszFormat, ... ) {
    va_list argptr;
    va_start( argptr, lpszFormat );
    size_t n = 1024;
    const char * format = lpszFormat;
    char buf[n + 1];
    char * s = buf;
    int iLen = vsnprintf( s, n, format, argptr );
    va_end(argptr);
    if ( iLen <= 0 ) {
        std::string sEmpty;
        mss = sEmpty;
        return  *this;
    }
    std::string sRes;
    sRes = s;
    mss = sRes;
    return  *this;
}

class TErr
{
public:

    TErr() { ; }
    TErr( TErr& EP ) { msMsg = EP.msMsg; }
    TErr( TStr& sMsg ) { msMsg = sMsg; }
    TErr&   operator=( TErr& sSrc ) { msMsg = sSrc.msMsg; return *this; }   // you
only need one of these 161028
    TStr    msMsg;
};

int main(int argc, char **argv)
{
    try {
        throw TErr( TStr().Format("PANIC NOT! Hello World %d", 99) );
    } catch ( TErr& e) {
        std::cout << "Hello World caught an error" << std::endl;
    //  std::cout << e.msMsg << std::endl;
    }
    std::cout << "Hello World" << std::endl;
    return 0;
}
Quuxplusone commented 7 years ago
This is still the same issue. TErrPub has a "copy" constructor that takes a non-
const parameter, which means it can't bind to the temporary TErrPub object that
is being thrown.

Note that this is ill-formed too, for the same reason:

  TErrPub x = TErrPub(sBS);

Presumably this code was originally written to compile with MSVC, which has a
long-standing bug that permits a non-const reference to bind to a temporary
object.

As I wrote in comment#2, I think there is a diagnostic quality bug here. We are
initializing a TErrPub object from the temporary object "TErrPub( sBS )", and
it is that copy which is failing. But the diagnostic makes it look like perhaps
the "TErrPub( sBS )" construction is itself failing. Perhaps something like
this would be clearer:

/media/FATSD29/Houston/wkspc/free/dev/wxhSLED/main.cpp:740:9: error: no
matching constructor for initialization of 'TErrPub' from temporary of type
'TErrPub'
                throw TErrPub( sBS );
                      ^~~~~~~~~~~~~~
../../../../include/TErrPub.h:15:2: note: candidate constructor not viable:
cannot bind non-const reference to temporary object in 1st argument
        TErrPub( TErrPub& EP );
        ^
../../../../include/TErrPub.h:17:2: note: candidate constructor not viable: no
known conversion from 'TErrPub' to 'TString &' for 1st argument
        TErrPub( TString& sMsg );
/media/FATSD29/Houston/wkspc/free/dev/wxhSLED/main.cpp:740:3: note: in
initialization of exception object of type 'TErrPub'
                throw TErrPub( sBS );
                ^
Quuxplusone commented 7 years ago
... and you can fix this invalid code by replacing

 TErrPub( TErrPub& EP );

with

 TErrPub( const TErrPub& EP );

and likewise for the other copy constructors.
Quuxplusone commented 7 years ago
[SOLVED]

Thanks Richard and Anton.  Now hundreds of lines of throw do NOT need to be
changed!  My understanding of const needs some work, Richard your fix works!
You were also right about microsoft allowing such things.  Here is a working
console program that demonstrates the discussion.  The only dependency is
iostream:

#include <iostream>

class TStr {
public:
    TStr()  { ; }
    TStr( const char * cp ) : mss(cp) { ; }     // string literal
    TStr( TStr& s ) { mss = s.mss; }        // copy ctor

//  TStr&   operator=( TStr& sSrc ) { mss = sSrc.mss; return *this; }
    TStr&   operator=( const TStr& sSrc ) { mss = sSrc.mss; return *this; }
//  TStr& Format( char * cpFmt, ...);
    TStr& Format( const char * cpFmt, ...);
    std::string mss;
};

TStr& TStr::Format( const char * lpszFormat, ... ) {
    va_list argptr;
    va_start( argptr, lpszFormat );
    size_t n = 1024;
    const char * format = lpszFormat;
    char buf[n + 1];
    char * s = buf;
    int iLen = vsnprintf( s, n, format, argptr );
    va_end(argptr);
    if ( iLen <= 0 ) {
        std::string sEmpty;
        mss = sEmpty;
        return  *this;
    }
    std::string sRes;
    sRes = s;
    mss = sRes;
    return  *this;
}

class TErr
{
public:

    TErr() { ; }
//  TErr( TErr& EP ) { msMsg = EP.msMsg; }
    TErr( const TErr& EP ) { msMsg = EP.msMsg; }
    TErr( TStr& sMsg ) { msMsg = sMsg; }
    TErr&   operator=( TErr& sSrc ) { msMsg = sSrc.msMsg; return *this; }
    TStr    msMsg;
};

int main(int argc, char **argv)
{
    try {
        throw TErr( TStr().Format("PANIC NOT! Hello World %d", 99) );
    } catch ( TErr& e) {
        std::cout << "Hello World caught an error" << std::endl;
    //  std::cout << e.msMsg << std::endl;
        std::cout << e.msMsg.mss << std::endl;
    }
    std::cout << "Hello World" << std::endl;
    return 0;
}