Closed Bug 208314 Opened 21 years ago Closed 21 years ago

MSVC++ .net 2003: Requires new Standard C++ Library (iostream, iomanip, fstream, et al)

Categories

(SeaMonkey :: Build Config, defect)

Product:
SeaMonkey
Component:
Build Config
Platform:
x86
Windows 2000
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: ian, Assigned: leaf)

References

Details

Attachments

(4 files, 4 obsolete files)

Simple patch
21.49 KB, patch
Details | Diff | Splinter Review
unbitrot vs2003 patch
28.46 KB, patch
Details | Diff | Splinter Review
Rough |using std::| patch
17.66 KB, patch
Details | Diff | Splinter Review
updated patch
121.90 KB, patch
timeless
: review+
dbaron
: superreview+
Details | Diff | Splinter Review 
Console:
--------------------------------------------------------------------------------
E:\Trees\Mozilla.1\mozilla>make -f client.mk build
cd /cygdrive/e/Trees/Mozilla.1/mozilla
/cygdrive/e/Trees/Mozilla.1/mozilla/configure
loading cache ./config.cache
checking host system type... i686-pc-cygwin
checking target system type... i686-pc-cygwin
checking build system type... i686-pc-cygwin
checking for gcc... cl
checking whether the C compiler (cl  ) works... yes
checking whether the C compiler (cl  ) is a cross-compiler... no
checking whether we are using GNU C... no
checking whether cl accepts -g... no
checking for c++... cl
checking whether the C++ compiler (cl  ) works... yes
checking whether the C++ compiler (cl  ) is a cross-compiler... no
checking whether we are using GNU C++... no
checking whether cl accepts -g... no
checking for ranlib... ranlib
checking for ml... /cygdrive/e/Program Files/Microsoft Visual Studio .NET 2003/V
C7/BIN/ml
checking for ar... ar
checking for ld... link
checking for strip... strip
checking for windres... windres
/cygdrive/e/Program: not found
checking for midl... midl
configure: error: $(CXX) test failed.  You must have MS VC++ in your path to bui
ld.
*** Fix above errors and then restart with "make -f client.mk build"
make: *** [/cygdrive/e/Trees/Mozilla.1/mozilla/Makefile] Error 1
--------------------------------------------------------------------------------

config.log:
--------------------------------------------------------------------------------
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.

configure:851: checking host system type
configure:872: checking target system type
configure:890: checking build system type
configure:1896: checking for gcc
configure:2009: checking whether the C compiler (cl  ) works
configure:2025: cl -o conftest    conftest.c  1>&5
Microsoft (R) 32-bit C/C++ Standard Compiler Version 13.10.3077 for 80x86

Copyright (C) Microsoft Corporation 1984-2002. All rights reserved.



conftest.c

Microsoft (R) Incremental Linker Version 7.10.3077

Copyright (C) Microsoft Corporation.  All rights reserved.



/out:conftest.exe 

/out:conftest.exe 

conftest.obj 

configure:2051: checking whether the C compiler (cl  ) is a cross-compiler
configure:2056: checking whether we are using GNU C
configure:2065: cl -E conftest.c
Microsoft (R) 32-bit C/C++ Standard Compiler Version 13.10.3077 for 80x86

Copyright (C) Microsoft Corporation 1984-2002. All rights reserved.



conftest.c

configure:2084: checking whether cl accepts -g
configure:2120: checking for c++
configure:2152: checking whether the C++ compiler (cl  ) works
configure:2168: cl -o conftest    conftest.C  1>&5
Microsoft (R) 32-bit C/C++ Standard Compiler Version 13.10.3077 for 80x86

Copyright (C) Microsoft Corporation 1984-2002. All rights reserved.



conftest.C

Microsoft (R) Incremental Linker Version 7.10.3077

Copyright (C) Microsoft Corporation.  All rights reserved.



/out:conftest.exe 

/out:conftest.exe 

conftest.obj 

configure:2194: checking whether the C++ compiler (cl  ) is a cross-compiler
configure:2199: checking whether we are using GNU C++
configure:2208: cl -E conftest.C
Microsoft (R) 32-bit C/C++ Standard Compiler Version 13.10.3077 for 80x86

Copyright (C) Microsoft Corporation 1984-2002. All rights reserved.



conftest.C

configure:2227: checking whether cl accepts -g
configure:2261: checking for ranlib
configure:2293: checking for ml
configure:2334: checking for ar
configure:2369: checking for ld
configure:2404: checking for strip
configure:2439: checking for windres
configure:2528: checking for midl
configure:2577: cl -c  -TC -nologo  conftest.c 1>&5
conftest.c

configure:2602: cl -c  -TP -nologo  conftest.C 1>&5
conftest.C

configure(2596) : fatal error C1083: Cannot open include file: 'iostream.h': No
such file or directory

configure: failed program was:
#line 2595 "configure"
#include "confdefs.h"
#include <iostream.h>
int main() {
 cout << "Hello World" << endl; 
; return 0; }
--------------------------------------------------------------------------------

Environment:
--------------------------------------------------------------------------------
ALLUSERSPROFILE=E:\Documents and Settings\All Users
APPDATA=E:\Documents and Settings\Ian Hickson\Application Data
CommonProgramFiles=E:\Program Files\Common Files
COMPUTERNAME=ABYSSINIAN
ComSpec=E:\WINNT\system32\cmd.exe
DevEnvDir=E:\Program Files\Microsoft Visual Studio .NET 2003\Common7\IDE
FrameworkDir=E:\WINNT\Microsoft.NET\Framework
FrameworkSDKDir=E:\Program Files\Microsoft Visual Studio .NET 2003\SDK\v1.1
FrameworkVersion=v1.1.4322
GLIB_PREFIX=E:\Program Files\MozTools.net
HOME=E:\
HOMEDRIVE=E:
HOMEPATH=\
INCLUDE=E:\Program Files\Microsoft Visual Studio .NET 2003\VC7\ATLMFC\INCLUDE;E:
\Program Files\Microsoft Visual Studio .NET 2003\VC7\INCLUDE;E:\Program Files\Mi
crosoft Visual Studio .NET 2003\VC7\PlatformSDK\include\prerelease;E:\Program Fi
les\Microsoft Visual Studio .NET 2003\VC7\PlatformSDK\include;E:\Program Files\M
icrosoft Visual Studio .NET 2003\SDK\v1.1\include;E:\Program Files\Microsoft Vis
ual Studio .NET 2003\SDK\v1.1\include\;E:\Program Files\Microsoft.NET\FrameworkS
DK\include\;E:\Program Files\Microsoft Visual Studio .NET\Vc7\include\
LIB=E:\Program Files\Microsoft Visual Studio .NET 2003\VC7\ATLMFC\LIB;E:\Program
 Files\Microsoft Visual Studio .NET 2003\VC7\LIB;E:\Program Files\Microsoft Visu
al Studio .NET 2003\VC7\PlatformSDK\lib\prerelease;E:\Program Files\Microsoft Vi
sual Studio .NET 2003\VC7\PlatformSDK\lib;E:\Program Files\Microsoft Visual Stud
io .NET 2003\SDK\v1.1\lib;E:\Program Files\Microsoft Visual Studio .NET 2003\SDK
\v1.1\Lib\;E:\Program Files\Microsoft Visual Studio .NET\Vc7\lib\;E:\Program Fil
es\Microsoft.NET\FrameworkSDK\Lib\
LIBIDL_PREFIX=E:\ProgramFiles\MozTools.net
LOGONSERVER=\\ABYSSINIAN
MOZ_TOOLS=E:\ProgramFiles\MozTools
MSVCDir=E:\Program Files\Microsoft Visual Studio .NET 2003\VC7
NUMBER_OF_PROCESSORS=1
OS=Windows_NT
Os2LibPath=E:\WINNT\system32\os2\dll;
Path=E:\Cygwin\bin;E:\ProgramFiles\MozTools.net\bin;E:\ProgramFiles\MozTools\bin
;E:\Program Files\Microsoft.NET\FrameworkSDK\Bin\;E:\Program Files\Microsoft Vis
ual Studio .NET\Common7\IDE\;E:\WINNT\Microsoft.NET\Framework\v1.0.3705\;E:\Prog
ram Files\Microsoft Visual Studio .NET\Vc7\bin\;E:\WINNT\system32;E:\WINNT;E:\WI
NNT\System32\Wbem;E:\Program Files\Common Files\Adaptec Shared\System;"E:\Progra
m Files\Hummingbird\Connectivity\7.00\Accessories\";E:\Program Files\VDMSound\;E
:\Program Files\Microsoft Visual Studio .NET 2003\Common7\IDE;E:\Program Files\M
icrosoft Visual Studio .NET 2003\VC7\BIN;E:\Program Files\Microsoft Visual Studi
o .NET 2003\Common7\Tools;E:\Program Files\Microsoft Visual Studio .NET 2003\Com
mon7\Tools\bin\prerelease;E:\Program Files\Microsoft Visual Studio .NET 2003\Com
mon7\Tools\bin;E:\Program Files\Microsoft Visual Studio .NET 2003\SDK\v1.1\bin;E
:\WINNT\Microsoft.NET\Framework\v1.1.4322;
PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH
PROCESSOR_ARCHITECTURE=x86
PROCESSOR_IDENTIFIER=x86 Family 6 Model 11 Stepping 1, GenuineIntel
PROCESSOR_LEVEL=6
PROCESSOR_REVISION=0b01
ProgramFiles=E:\Program Files
PROMPT=$P$G
SystemDrive=E:
SystemRoot=E:\WINNT
TEMP=E:\DOCUME~1\IANHIC~1\LOCALS~1\Temp
TMP=E:\DOCUME~1\IANHIC~1\LOCALS~1\Temp
USERDOMAIN=ABYSSINIAN
USERNAME=Ian Hickson
USERPROFILE=E:\Documents and Settings\Ian Hickson
VCINSTALLDIR=E:\Program Files\Microsoft Visual Studio .NET 2003
VDMSPath=E:\Program Files\VDMSound\
VS71COMNTOOLS=E:\Program Files\Microsoft Visual Studio .NET 2003\Common7\Tools\
VSINSTALLDIR=E:\Program Files\Microsoft Visual Studio .NET 2003\Common7\IDE
windir=E:\WINNT
--------------------------------------------------------------------------------
Changing the CXX test in configure.in to:

        AC_LANG_CPLUSPLUS
        AC_TRY_COMPILE([#include <iostream>],
            [ std::cout << "Hello World" << std::endl; ],,
            AC_MSG_ERROR([\$(CXX) test failed.  You must have MS VC++ in your
path to build.]) )
        AC_LANG_RESTORE

...worked (i.e. remove the .h from the include filename, add the std:: prefix to
the identifiers from that namespace).
I can confirm this problem using MSVC++ .NET 2003. I also have MSVC++ .NET 2002,
and am able to compile successfully using that.

More useful info:
http://www.mozillazine.org/forums/viewtopic.php?t=12526
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore/html/_core_Differences_in_iostream_implementation.asp
A number of files use iostream.h:

   http://lxr.mozilla.org/seamonkey/search?string=iostream.h

In my build I'm just going to replace <iostream.h> with <iostream>. I don't know
how we want to fix this on the trunk though.
Summary: MSVC++ .net 2003: #include <iostream.h> test fails → MSVC++ .net 2003: #include <iostream.h> no worky
Attached patch Simple patch — — Splinter Review 
This patch simply replaces all occurances of

   #include <iostream.h>

...with:

   #include <iostream>
   use namespace std;

It was done using a single perl command over the files mentioned in the lxr
results above. It also includes the aforementioned configure.in changes, mixed
in with some changes for bug 208320 and bug 208461.
The above patch works.
mozilla\configure.in still has two references to iostream.h in it:

AC_MSG_CHECKING(for ios::binary)
AC_CACHE_VAL(ac_cv_ios_binary,
 [AC_TRY_COMPILE([#include <iostream.h>]

and

AC_MSG_CHECKING(for ios::bin)
AC_CACHE_VAL(ac_cv_ios_bin,
 [AC_TRY_COMPILE([#include <iostream.h>]
I have opened up a bug to deal with remaining issues to getting Moz to build on
MSVC++ 2003:

http://bugzilla.mozilla.org/show_bug.cgi?id=215224

BTW, there needs to be a change to "configure" also to get it working. This also
deals with the midl version.

Has anyone here gotten Moz to build all of the way on 2003?
Comment on attachment 125062 [details] [diff] [review]
Simple patch

A few comments here:
 * some of the compilers we support probably won't support #include <iostream>
 * we don't want to bring in all of the |std| namespace -- just the things we
need.  IIRC, there's more namespace clutter in it than in the stuff that was
brought in by the old-style headers.
This brings up the trunk with vs2003, but still isn't what would be
accepted for the tree.
Comment on attachment 131763 [details] [diff] [review]
unbitrot vs2003 patch

It would probably be good to separate the MIDL_FLAGS changes into a separate
patch, since that could go in.

Are the diffs in directory/c-sdk just the result of diffing a srcdir build?

I'm wondering what compilers using the new-style headers and |using| would
break.	Probably not many.  And we could probably use some macros like NEW_H
(perhaps a CPP_STD_H() macro).	If we do that, I'd rather bring in only what we
need with |using| rather than a complete |using namespace std|, although I'd be
interested to know what others think about that issue.	And if there were 
tier-1 platforms that wouldn't need the |using| declarations, we might just
want to use the |using namespace std| to avoid bustage when people check in
from those platforms and forget the neccesary (perhaps macro-ized) |using|.
MIDL changes are seperated out in bug 208461.
A simple testcase is:

#include <iostream>
using std::cout;
using std::endl;
int main() { cout << "hello" << endl; return 0; }

This compiles on egcs-2.91.66 and gcc-2.96-118.7.2, and gcc 3.2.2.


The testcase that shows what could lead to bustage on newer compilers is:

#include <iostream>
int main() { cout << "hello" << endl; return 0; }

This compiles, unfortunately, on egcs-2.91.66 and gcc-2.96-118.7.2.


I'm curious what happens on VC++ 6.0.
The patch doesn't doing anything with 'configure'. Is there some way to generate
a new one from 'configure.in' or something? The last time I had it working, I
just hand edited 'configure' which sucked.

If there is no way to generate a new one, please post a patch for 'configure'.
configure is a file automatically generated from configure.in. If I recall 
correctly, the program which does it is autoconf. I would guess that configure 
actually says this in comments at the top somewhere.
OK, I got autoconf to work. Now I am getting this:

$ make -f client.mk build_all_depend
cd /cygdrive/c/mozilla
/cygdrive/c/mozilla/configure
loading cache ./config.cache
checking host system type... i686-pc-cygwin
checking target system type... i686-pc-cygwin
checking build system type... i686-pc-cygwin
checking for gcc... cl
checking whether the C compiler (cl  ) works... no
configure: error: installation or configuration problem: C compiler cannot creat
e executables.
*** Fix above errors and then restart with "make -f client.mk build"
make: *** [/cygdrive/c/mozilla/Makefile] Error 1

This is after I applied the patch, and then did autoconf.
Nevermind, I think it's something with my environment variables. It has to be.
Sorry for the spam.
For completeness' sake, I should note that it's not necessarily just iostream.h
that doesn't work. The entire Standard C++ Library has been upgraded. Many other
functions such as iomanip.h won't work either. See here for more details:

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore/html/_core_port_to_the_standard_c.2b2b_.library.asp
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore/html/_core_differences_in_iostream_implementation.asp

From my (limited) understanding, I think the challenge is to find a solution
that works on MSVC++.NET 2003, as well as VC6, which does not support the newer
Standard C++ Libraries. Maybe other compilers don't support these new set of
functions as well, I'm not sure.
Assuming that MSVC++ .NET 2003 has an identifier that can be queried, a possible
solution is to use an #ifdef whereever there is an <iostream.h> to provide
MSVC++ .NET 2003 <iostream> and <iostream.h> to all other compilers.  Yes, I
realize this  is a messy way of doing it, but it would solve the problem of
allowing MSVC++ .NET 2003 to build it without breaking other compilers that
don't support <iostream>.

Just looking at the environment varibles defined in a MSVC++ .NET 2003 command
prompt, VS71COMNTOOLS appears to qualify as a varible that could be queried for
its existance since .NET 2003 is v7.1.

Actually, this it quite similar to David Baron's idea.  His method of using
macros would probably be cleaner though (although I'm not too familiar with
macros in C++).
The macro _MSC_VER is 1310 for VC.NET 2003.  (It's 1300 for VC.NET 2002, 1200
for VC++ 6.0, and 1100 for VC++ 5.0.  See bug 208439.)

However, presumably the new-style headers work on older versions.  They also
work on many other compilers.  Furthermore, when we move to solutions like this,
we should use the standard form on as many compilers as possible, and the
nonstandard form on as few as possible.
So, if I understand this correctly, we should try to compile a list of what
platform/compiler combinations break using the standard-compliant (new style)
headers?

That would allow us to make a patch that moves towards the standards compliant
headers and not break older compilers that we still support.
Except for VC++ on Windows, it would be done using a configure.in test, so
there's no need for a list, except for knowing which versions of VC++ on Windows
do what.
Here's a list of what versions of VC++ do what.

VC++ 6 and lower: new style headers UNSUPPORTED
VC++ .NET 2002: new style headers SUPPORTED
VC++ .NET 2003: new style headers REQUIRED
COmment #22:
If my memory serves me correctly, VC6 supports current standard C++ header names
(<iostream>).

VC++ 6 and lower: old style headers SUPPORTED
VC++ .NET 2002: old style headers SUPPORTED but almost DEPRECATED
VC++ .NET 2003: old style headers DEPRECATED and UNSUPPORTED
Mass reassign of Build/Config bugs to Leaf.
Assignee: mozbugs-build → leaf
As a side note, I am trying to use stlport instead of vc2003. Its a free "good"
impl of the stl & friends. though I have had some problems with this so far.

http://www.stlport.org

might be a short term workaround if I can get it working.
Keywords: helpwanted
Am updating the summary to widen the scope of this bug slightly, so that the aim
of the bug is the bring Mozilla in compliance with the new Standard C++ Library,
which is deprecated from VS.NET 2003. VC6 *does* support this new library set,
so backwards compatibility is not an issue with VC6.

Basically, the problem isn't only in iostream.h, but also in iomanip.h, and
fstream.h (and possibly others), which have all been deprecated. We need to
updated the headers to the new style for these, and all other old headers.

They're not deprecated from GCC yet, but even GCC issues warnings to the effect
that they are deprecated/antiquated headers.

biesi suggested that if we don't want to use the entire std namespace, we can
use something like:

using std::cout; using std::endl;

to limit what namespace items we introduce. This should hopefully alleviate that
problem.

With the issue of VC6 breaking with these changes no longer an issue, it would
be great if we could get this into Mozilla 1.7a.
Summary: MSVC++ .net 2003: #include <iostream.h> no worky → MSVC++ .net 2003: Requires new Standard C++ Library (iostream, iomanip, fstream, et al)
logparse.cpp is still broken; ::nocreate doesn't exist in the standard for
<iostream> and ipfx() broke on sodapop3k's gcc.
I'm not certain I caught all of the need |using std::| users in prstrms.cpp and
the file uses ::nocreate as well (and probably other non-standard items).
Here is a simpler approach for those who want to build while waiting for a patch.

Just create a the following 4 header files and put them in a direcory in your
INCLUDE path.

File 1 fstream.h:

#include <fstream>
using std::fstream;
using std::ifstream;
using std::ofstream;

File 2 iomanip.h:

#include <iomanip>

File 3 iostream.h:

#include <iostream>
using std::cerr;
using std::cout;
using std::endl;
using std::flush;
using std::dec;
using std::hex;
using std::ios;
using std::istream;
using std::ostream;
using std::setbuf;
using std::streambuf;
using std::streamoff;
using std::streampos;

File 4 strstream.h:

#include <strstream>
using std::istrstream;


copied and pasted the contents straight into to 4 files with those names, put
those .h files into my vc7\include folder and tried to compile.  it gets as far
as nsiconsolelistener and then stops with something crashing.
You're using the binaries from bug 208345, right?
oops didnt see that.  stuck them in now.
nope i stuck them over the top of the files in my vc folder and its still doing it
probly help if i stuck em in the right place i guess :P

they really need an edit button on here.
William's method from comment 28 seems to work fine for Firebird. I just built
using this method, and the build completed successfully without problems, and
the browser seemed to work fine.

This has now become my new build method for building with VS.NET 2003 until this
bug gets fixed.
Comment on attachment 136294 [details] [diff] [review]
old patch that gets rid of most of the *stream usage

This looks good to me, but you should probably run it by one of the XSLT folks
(sicking?) and perhaps darin as well.  (I'll r+sr the rest once you do that.)
This patch will give a tree buildable with vs.net2003.	Not entirely
sure this is the best solution.
Attachment #136294 - Attachment is obsolete: true
tor, can you kill the duplicate unknwn include in TestCOM?
Attachment #136318 - Attachment is obsolete: true

    
    

    
  
Comment on attachment 139120 [details] [diff] [review]
remove *stream patch update to tip

I looked over this quickly, and I delegate the remainder of my sr= to gcc.  If
you build this with a recent version of gcc 3.x that gives printf format
warnings, and you check that you haven't introduced any such warnings in the
files you've modified, sr=dbaron.

sicking should probably review the transformiix change.

        Attachment #139120 -
        Flags: superreview+

        Attachment #139120 -
        Flags: review?(bugmail)

    
    

    
  


  

        Attachment #139120 -
        Attachment is obsolete: true

    
    

    
  
timeless' comments on attachment 139277 [details] [diff] [review]:

<timeless> RCS file:
/cvsroot/mozilla/intl/compatibility/tests/TestI18nCompatibility.cpp,v
<timeless> @@ -66,7 +66,7 @@
<timeless> -        cout << csid << " " << tempCstr << "\n";
<timeless> +        printf("%us %s\n", csid, tempCstr);
<timeless>          delete [] tempCstr;
<timeless>          ^^^^^^^^^ wrong freeing function
<timeless> i'm fairly certain it should be using nsMemory::Free

---

<timeless> RCS file: /cvsroot/mozilla/intl/locale/tests/LocaleSelfTest.cpp,v
<timeless> @@ -141,14 +141,14 @@
<timeless> +  fflush(NULL);
<timeless> shouldn't that be stdout?
<timeless> aebrahim: ok, i assert that the change for that file should use
(stdout) always

---

<timeless> RCS file: /cvsroot/mozilla/intl/lwbrk/tests/TestLineBreak.cpp,v
<timeless> +                   printf("[%d] expect %d but got %d\n", j, out[j],
res[j]);
<timeless> original text and new text (equivalent) have bad wording..
<timeless> should probably be 'expected' ...
<timeless> +            printf("WARNING!!! return size wrong, expect %d bet got
%d\n",
<timeless> +                   outlen, i);
<timeless> 'bet' should be 'but' (still notes to self/weirdal)

---

<timeless> RCS file: /cvsroot/mozilla/intl/strres/tests/StringBundleTest.cpp,v
<timeless> at least in @@ -158,7 +158,7 @@
<timeless>    value = ToNewCString(v);
<timeless> appears to be leaked

---

Index: intl/unicharutil/tests/UnicharSelfTest.cpp
===================================================================
RCS file: /cvsroot/mozilla/intl/unicharutil/tests/UnicharSelfTest.cpp,v
<timeless> -  cout << "ISO-8859-1 " <<
"attr_FallbackHexNCR+attr_EntityAfterCharsetConv " << "html40Latin1 " << "\n";
<timeless> +  printf("ISO-8859-1 attr_FallbackHexNCR+attr_EntityAfterCharsetConv
html40Latin1\n");
<timeless> the old code had a random trailing space

---

<timeless> RCS file: /cvsroot/mozilla/netwerk/test/PropertiesTest.cpp,v
<timeless> @@ -116,7 +116,7 @@
<timeless>        delete[] value;
<timeless>        ^^^^^^^^^ wrong freeing function
<timeless> i'm fairly certain it should be using nsMemory::Free

---

<timeless> RCS file: /cvsroot/mozilla/netwerk/test/PropertiesTest.cpp,v
<timeless> @@ -163,7 +163,7 @@
<timeless> -    cout << key.get() << "\t" << value.get() << endl;
<timeless> +    printf("%s\t %s\n", key.get(), value.get());
<timeless> it /appears/ that there might not be a reason for the space after \t

---

<timeless> Index: xpcom/tests/TestCOMPtr.cpp
<timeless> library closed
<timeless> "Have a good evening"

I think timeless will take a look at the rest of the patch later.

    
    

    
  
RCS file: /cvsroot/mozilla/xpcom/tests/TestCOMPtr.cpp,v
@@ -195,34 +199,34 @@
 CreateIFoo( void** result )
     // a typical factory function (that calls AddRef)
   {
+    printf(">>CreateIFoo() --> ");
     IFoo* foop = new IFoo;
+    printf("IFoo@%p\n", STATIC_CAST(void*, foop));
 
     foop->AddRef();

should really check for new failing in factories (to avoid the problem where
people copy bad code)...
and again in
@@ -287,14 +291,14 @@
 CreateIBar( void** result )

@@ -466,86 +470,86 @@
...
 #if 0
 			if ( foo1p == 1 )
+				printf("foo1p allowed compare with in\n");
i think "in" should be "int".

RCS file: /cvsroot/mozilla/xpcom/tests/TestHashtables.cpp,v
@@ -114,103 +113,103 @@
 
 PLDHashOperator
 nsTEnumGo(EntityToUnicodeEntry* aEntry, void* userArg) {
+  printf("  enumerated \"%s\" = %s\n", 
+         aEntry->mNode->mStr, aEntry->mNode->mUnicode);

Isn't mUnicode a PRUint32 and not a char*?

(you treat it as %s a few times, and then switch to %u around @@ -445,66 +449,66 @@)

Again, 
@@ -343,14 +347,14 @@
 CreateIFoo( IFoo** result )
     // a typical factory function (that calls AddRef)
has the bad factory sample code (doesn't check for new failure)

RCS file: /cvsroot/mozilla/xpcom/tests/TestObserverService.cpp,v
+void printString(nsString &str) {
     const char *cstr = ToNewCString(str);
+    printf("%s", cstr);
     delete [] (char*)cstr;
 }

ToNewCString/delete[] isn't a matched pair, perhaps just printf("%s",
NS_ConvertUTF16toUTF8(str).get()) ? [or LossyCovert if you have some preference
for stdout]

    
    

    
  

      
      
      
      

        Attached patch
          
          
        updated patch
        — 
          — Splinter Review
      

    


  
Thanks for looking through the patch.  I've applied the changes that relate
to the actual output and do not change the output from what the current
versions do.  If you want to fix the other things, please file another
bug.

        Attachment #139277 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 140451 [details] [diff] [review]
updated patch

note to self: thank jkeiser for attachment diffing

        Attachment #140451 -
        Flags: review+

    
  
Keywords: helpwanted

    
  

        Attachment #140451 -
        Flags: superreview?(dbaron)

        Attachment #140451 -
        Flags: superreview?(dbaron) → superreview+

    
    

    
  
Checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

    
    

    
  
Hmm. It seems to me that moving to printf is a step backward.
It removes the type safety aspect of the programming. How about
something like the following?

class NotPrintf {
public:
  NotPrintf & operator<<(int n) { printf("%d",n); };
  // etc.
};

What do you think?

Mark Stankus

    
    

    
  
I mean

class NotPrintf {
public:
  NotPrintf & operator<<(int n) { printf("%d",n); return *this;};
  // etc.
};

    
    

    
  
We already only used printf in test apps.  Using iostream requires that we link
to the full C++ standard library, which we currently don't need to, and it
probably is also bad for code size.  Furthermore, gcc warns when printf has
format mismatches.

    
    

    
  
(In reply to comment #50)
> We already only used printf in test apps.  Using iostream requires that we link
> to the full C++ standard library, which we currently don't need to, and it
> probably is also bad for code size.  Furthermore, gcc warns when printf has
> format mismatches.


Ok. gcc is better with checking printf than I would have guessed.

The solution proposed in Comments #48,49 does *not* require iostream to be used.
It is just overloading "operator <<".
Product: Browser → Seamonkey

    
  
Blocks: majorbugs

    
  
No longer blocks: majorbugs

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: