[boost] Re: Detection of wchar_t for Intel compiler

From: Guillaume Melquiond (guillaume.melquiond_at_[hidden])
Date: 2004-01-28 06:20:42

Le mar 27/01/2004 à 19:42, Giovanni Bajo a écrit :
> Guillaume Melquiond wrote:
> 
> > There was a recent change to the config/compiler/intel.hpp file in
> > order  to change (one more time) the way the detection of wchar_t works.
> 
> I did it, and discussed it on this very list.
> 
> > So I've committed a trivial fix to CVS head. However it was a bit
> > annoying to note that a patch had been committed to such an important
> > place carefreely (otherwise the bug would have been found). This is my
> > first concern: the configuration of a compiler had been changed
> > without even being tested.
> 
> You're implying way too much. There was a typo on my part, sure, but it wasn't
> detected under Windows. I *did* test it, and we did discuss the patch on the
> list. I never commit untested patches.

No I wasn't implying too much. I never said you didn't test at all your
patch, I said you didn't test your patch "on this compiler/platform"
(this is a quote from my previous mail).

The Windows and Linux front-ends of the Intel compiler are completely
different. The first was designed in order to be compatible with
Microsoft compiler, the second was designed in order to be compatible
with GNU compiler. They are not the same compilers at all (just take a
look to the macros defined for both versions in the regression tests,
there are huge differences).

I wouldn't have written such a long mail if you just had modified the
part of the configuration specific to the Windows front-end or the part
common to both front-ends. However, you did modify the part of the
configuration specific to the Linux front-end. And such a patch couldn't
be tested by using the Windows compiler (since it was specific to the
Linux compiler). That's what I was pointing out: you modified the
configuration of a compiler you couldn't test with.

> > Second concern: since the patch was not tested on this
> > compiler/platform configuration, is it sure it behaves sanely?
> 
> Please, review our discussion on the topic.

I will suppose you mean yes by this answer.

> > Sincethere was a lot of
> > changes in this area, shouldn't a specific test file be added in order
> > to finally ensure the correct detection of wchar_t? I can also
> > directly exercise some testcases if provided; I can test with ICC 7
> > and 8 for Linux.
> 
> We need to do a detection through the preprocessor.
> 
> > Finally, my third concern: shouldn't such changes be committed to the
> > release branch since they directly involve the configuration of an
> > existing compiler?
> 
> Again, please review the discussion. I told that I would commit it to the
> release branch after a few days, to not destabilize the branch. In fact, I was
> awaiting fallouts on different platforms, just like the one you fixed. If I had
> committed it to the release branch immediatly, I would have broken it as well.

Boost is currently in the last stage before release. If you want the
patch to be included in the 1.31 release, you need to push it as soon as
possible in order for it to be at least in one release candidate. All
the regression-testing is currently done on the release branch and the
bug would have been caught a lot sooner. If you had just waited until
the last minute, the 1.31 would have simply been unusable with the Linux
version of the Intel compiler.

However, if people think it isn't 1.31 material, then it's just fine. I
don't have any strong opinion on this subject since, as I said, I don't
use wchar_t.

> > Sorry for this long mail, but since release time draws near, I wanted
> > to point out this problem.
> 
> Thank you for the fix.

Regards,

Guillaume