Re: wxWidgets 2.9 build

From: Peter Geoghegan <peter(dot)geoghegan86(at)gmail(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Guillaume Lelarge <guillaume(at)lelarge(dot)info>, pgadmin-hackers(at)postgresql(dot)org
Subject: Re: wxWidgets 2.9 build
Date: 2011-01-17 15:30:15
Message-ID: AANLkTimdpA=Q0A45pa66cH5791T--AiKHs5dQHHV_ieo@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On 16 January 2011 22:39, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> On Sun, Jan 16, 2011 at 6:47 PM, Peter Geoghegan
> From what I can see in a quick glance at the docs, the only conversion
> operator is to const char* in 2.8, so I guess that is a legacy thing
> as you suggest. There are good reasons for the different conversion
> members like c_str() and mb_str().

I meant that the co-existence of both implicit conversions and
redundant explicit functions must be a legacy issue. I appreciate that
there are good reasons for the variants like mb_str() though. In case
you didn't get the joke, I called it the three stooges idiom because
they used to attempt to all get through a door threshold at the same
time and block each other out.

>> Why does sysSettings::Write(const wxString&, const wxChar*) take a
>> const wxChar* as its second argument rather than just a wxString? I'm
>> inclined to just use a const wxString&, and avoid c strings if at all
>> possible, which resolves the ambiguity in various places. Obviously we
>> can't have null references, so I'm going to write workarounds where
>> that was possible before, where there might have been different
>> codepaths for NULL and so on.
>
> Excellent question - especially as it just calls wxConfig::Write,
> which is basically "bool wxConfigBase::Write(const wxString& key,
> const wxString& value)"
>
> So unless I'm missing something, it can just be changed.

Good. Already done.

>> in pgsDictionaryGen.cpp, why do we do this?:
>>
>>                        wxString line;
>>                        while ((line = text.ReadLine()) && !input.Eof())
>>                        {
>>                                ++result;
>>                        }
>>

> It's getting late and I've had an extremely stressful weekend, so
> maybe I'm missing something obvious but I don't see an issue there.
> It's reading from the stream (text) line by line into a dummy variable
> (line), and counting the lines until it gets to the end of the file
> (input).

Sorry to hear that. I just thought that it might not be prudent to
rely on the implicit cast, and that it superficially looked like the
classic "assignment operator in place of equality operator" mistake.
Perhaps I should have been clearer on that.

> How did the OGL port go? I looked at that briefly, and had a rough
> build in 10 minutes or so iirc. Oh, and in answer to your previous
> comments on the distribution; you're quite correct - the wxWidgets
> licence isn't suitable for distribution with pgAdmin. We'd have to get
> the patch applied upstream, or host our own port. If we need to do the
> latter, then I guess we should also setup our own GIT repo.

I didn't look at it yet. I thought that it would be better to leave it
until there was a consensus on how to deal with it, and just tackle
the landslide of other compiler errors. Hopefully the fact that
compilation of certain TUs is terminated when ogl.h can't be included
isn't masking a whole lot more errors at this point. Have we
considered how maintaining OGL ourselves will affect package
maintainers and so on?

IANAL, but it seems like a separate git repo is the way to go to
protect ourselves from copyleft implications. It might be worth asking
the author/maintainer to change the licence. That's highly unlikely to
happen, but it's still worth asking the question.

We're down to just 25kb of make output when I haven't done a make
clean first; otherwise, there'd be some additional warnings which I'll
fix later. Plus, as I said, I still haven't looked at OGL, and we can
take it that once we've dealt with that issue, more unrelated errors
will occur as we get through those additional TUs.

The fact that wxString::c_str() now returns a special proxy object is
a pain. Because it doesn't have a compiler generated copy constructor
or is composed of objects that don't, you cannot pass it to vararg
functions, of which there seems to be a few. I'm cleaning that up by
explicitly casting to const wxChar* (and generally not calling c_str()
and not going through the proxy object). That seems to work fine.
However, it's not immediately clear how to do this in the bison
grammar file pgsParser.yy; changing something like this:

@@ -886,8 +886,8 @@ assign_element
| PGS_IDENTIFIER '[' expression ']' '[' expression ']'
PGS_EQ_OP expression
{

wxLogScriptVerbose(wxT("SET %s[%s][%s] = %s"),
-
$1->c_str(), $3->value().c_str(),
-
$6->value().c_str(), $9->value().c_str());
+
((const wxChar*) $1), ((const wxChar*) $3->value()),
+
((const wxChar*) $6->value()), ((const wxChar*)
$9->value()));

doesn't fix the problem. I still see the same error, like
"pgscript/pgsParser.yy:494:92: error: cannot pass objects of
non-trivially-copyable type ‘class wxCStrData’ through ‘...’".

This code in ctlSQLBox::RegexFindText() errors:

wxWX2MBbuf buf = (wxWX2MBbuf)wx2stc(text);

wx2stc is now in private.h . See
http://groups.google.com/group/wx-dev/browse_thread/thread/1477c44701f792ea/8205f32f8433b03b
and please advise.

For your information, I've attached the text of remaining errors. A
few mysteries remain, such as why some classes that we were
instantiating have become abstract. I haven't really looked into that
yet. I'm going to give OGL some attention now.

--
Regards,
Peter Geoghegan

Attachment Content-Type Size
29_build_errors_ignore.txt text/plain 25.5 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Guillaume Lelarge 2011-01-17 22:13:28 Re: wxWidgets 2.9 build
Previous Message Dave Page 2011-01-16 22:39:39 Re: wxWidgets 2.9 build