wxWidgets 3.1.5

[Miguel Gimenez]

Can you check the attached patch?

The code in that function is very strange, why use

Code Select Expand

m_path=wxString(m_fe->GetFullPath(ti).c_str());
instead of just

Code Select Expand

m_path=m_fe->GetFullPath(ti);
?

EDIT: around line 1260 there is an almost identical code snippet, you can still get an assert from it later.

[oBFusCATed]

Probably to break ref counting implementations and force an actual copy.

p.s. Is this code running on the main thread?
p.p.s. Please use wxString() instead of wxEmptyString.

[Miguel Gimenez]

FileExplorerUpdater::Update() and VCSFileLoader::Update() are called (or should be called) from the main thread and they start the threads:

Code Select Expand


    void Update(const wxTreeItemId &ti); //call on main thread to do the background magic
    void Update(const wxString &op, const wxString &source_path, const wxString &destination_path, const wxString &comp_commit); //call on main thread to do the background magic


[gd_on]

With this patch, as it is, no more assert message (until the next one  ). Thanks.
wxWidgets team say that there are other possible incompatibilities in version 3.1.5. I don't know if C::B is impacted.

Comment (by vadz):

I'm not sure where exactly is the code calling `GetString(-1)`, but it
should be fixed and not just hidden by disabling the asserts.

Note that this change was documented in the incompatible changes section
of the change log file:
{{{
  - wxChoice::GetString() now consistently asserts when passed an invalid
index.
}}}

--
Ticket URL: <https://trac.wxwidgets.org/ticket/19096#comment:4>

Why do you say wxEmptyString should be avoided ?

[Miguel Gimenez]

I have just created ticket 1079 with a patch.

[oBFusCATed]

Why do you say wxEmptyString should be avoided ?

Because it is more expensive probably.
I'm seeing lots of commits in wx where they are removing wxT and they are replacing wxEmptyString with wxString().

[sodev]

wxEmptyString is just a wxChar*, this might lead to some surprises in certain situations, but otherwise it might be a bit faster than wxString().

[oBFusCATed]

sodev: Why faster? wxString() is the fastest possible way to initialize an empty string. The wxString(wxChar *) is something that should work for actual strings.

[gd_on]

Is speed a real problem in that case ?
And in the official documentation :

Code Select Expand


wxString wxEmptyString

The global wxString instance of an empty string.

Used extensively in the entire wxWidgets API.


I have also seen that a few guys had problem with this wxEmptyString, but the doc seem to say it's good and used extensively. It's also said it's a wxString but effectively seen as a wxChar* inside C::B (which use it more than 600 times in it's workspace  :-[). so ?? not so bad !

[sodev]

oBFusCATed: It is (only?) faster in situations where no temporary wxString object gets created. Right now only comparision against wxEmptyString that use a wxChar* overload comes into my mind. Maybe also method parameter initialization benefits, depends if direct initialization of a wxString from a wxChar* is faster than copy/move-initialization from a temporary wxString.

[oBFusCATed]

I've not measured, nor I've done any deep investigations.
I guess the difference is the same as std::string() and std::string("")...
The latter needs to find the end of the char* and do the copy, so it is obviously more expensive.
Does it matter in the real world - no, 99.9999%.

[LETARTARE]

Where did you find this version 'wxWidget-3.1.5' ?

[gd_on]

I follow it on github : https://github.com/wxWidgets/wxWidgets

[oBFusCATed]

gd_on: It is best to describe this as wx-master and specify a commit.

[gd_on]

OK for wx-master.
It's effectively 3.1.5 today, but tomorrow, next week, ... it will probably be 3.2.0 !
For the commit, more difficult, it changes several times in a day ! ;D
And I build it very often ...