Closed Bug 818861 Opened 12 years ago Closed 12 years ago

Shouldn't be able to promise a flat string from a flat string

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla20

People

(Reporter: neil, Assigned: neil)

References

Details

Attachments

(2 files, 1 obsolete file)

Fixed patch
21.72 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
c-c fix
1.88 KB, patch
mconley
: review+
Details | Diff | Splinter Review 
Sometimes people erroneously think that you must use PromiseFlat(C)String if you want to call .get() on your string; most of our strings are already flat, the notable exceptions being nsA(C)String and nsDependent(C)Substring.
Attached patch Proposed patch (obsolete) — — Splinter Review 
Compiles on Windows, will send to Try.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Try run for 6ff2ce900550 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6ff2ce900550
Results (out of 69 total builds):
    success: 47
    warnings: 7
    failure: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-6ff2ce900550
Attached patch Fixed patch — — Splinter Review 
OK, this version seems to be passing try on non-Windows platforms too.
Attachment #689173 - Attachment is obsolete: true
Attachment #689401 - Flags: review?(dbaron)
Try run for 8e18a00fbb7f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=8e18a00fbb7f
Results (out of 77 total builds):
    success: 64
    warnings: 13
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-8e18a00fbb7f
Try run for e549fe7e4274 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e549fe7e4274
Results (out of 80 total builds):
    success: 62
    warnings: 7
    failure: 11
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-e549fe7e4274
Try run for 6ff2ce900550 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6ff2ce900550
Results (out of 70 total builds):
    success: 47
    warnings: 7
    failure: 16
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-6ff2ce900550
Try run for a1a7d3631a34 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a1a7d3631a34
Results (out of 111 total builds):
    success: 102
    warnings: 8
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-a1a7d3631a34
Comment on attachment 689401 [details] [diff] [review]
Fixed patch

In WebGLContextGL, add |const|s back where things were const before.

The nsGlobalWindow.cpp aren't related; drop them from this patch.

nsTPromiseFlatString.h:

>-   *   SomeOSFunction( PromiseFlatCString(aCString).get() ); // GOOD
>+   *   SomeOSFunction( PromiseFlatCString(aACString).get() ); // GOOD
>    *
>    * Here's a BAD use:
>    *
>-   *  const char* buffer = PromiseFlatCString(aCString).get();
>+   *  const char* buffer = PromiseFlatCString(aACString).get();

Use aCSubstring rather than aACString, since nsCSubstring is the
preferred name.


>+        // NOT TO BE IMPLEMENTED
>+      nsTPromiseFlatString_CharT( const string_type& str );

Mark this as MOZ_DELETE.  Also the 2 things right above it.

>-  // e.g., PromiseFlatCString(Substring(s))
>-inline
>+template<class T>
> const nsTPromiseFlatString_CharT
>-TPromiseFlatString_CharT( const nsTSubstring_CharT& frag )
>+TPromiseFlatString_CharT( const T& string )
>   {
>-    return nsTPromiseFlatString_CharT(frag);
>+    return nsTPromiseFlatString_CharT(string);
>   }
>-
>-  // e.g., PromiseFlatCString(a + b)
>-inline
>-const nsTPromiseFlatString_CharT
>-TPromiseFlatString_CharT( const nsTSubstringTuple_CharT& tuple )
>-  {
>-    return nsTPromiseFlatString_CharT(tuple);
>-  }

You should leave a comment that this is intentional, to make
things not compile where the relevant constructor won't compile.

r=dbaron with that

We should also crack down on the people using nsPromiseFlat[C]String's
constructor directly rather than PromiseFlat[C]String.
Attachment #689401 - Flags: review?(dbaron) → review+
Try run for 6d11a47a673a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6d11a47a673a
Results (out of 19 total builds):
    success: 18
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-6d11a47a673a
Attached patch c-c fix — — Splinter Review 

        Attachment #695254 -
        Flags: review?(mconley)

    
    

    
  
Comment on attachment 695254 [details] [diff] [review]
c-c fix

Looks fine. Thanks Neil!

        Attachment #695254 -
        Flags: review?(mconley) → review+

    
    

    
  
Try run for 6d11a47a673a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=6d11a47a673a
Results (out of 20 total builds):
    success: 18
    warnings: 1
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/neil@parkwaycc.co.uk-6d11a47a673a

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/5abe31fa4a5c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20

    
    

    
  
The patch in attachment 695254 [details] [diff] [review] seems to have caused the following:

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1356318210.1356321952.31757.gz#err0
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1356299242.1356303620.9685.gz#err0

nsMsgFilter.cpp
/usr/local/bin/ccache /builds/slave/comm-cen-trunk-osx64/build/mozilla/../clang/bin/clang++ -arch i386 -o nsImportMail.o -c  -fvisibility=hidden -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DZLIB_INTERNAL -DMOZ_SUITE=1 -DOSTYPE=\"Darwin\" -DOSARCH=Darwin  -I/builds/slave/comm-cen-trunk-osx64/build/mailnews/import/src -I. -I../../../mozilla/dist/include -I../../../mozilla/dist/include/nsprpub  `/builds/slave/comm-cen-trunk-osx64/build/objdir/i386/mozilla/dist/sdk/bin/nspr-config --prefix=/builds/slave/comm-cen-trunk-osx64/build/objdir/i386/mozilla/dist --includedir=/builds/slave/comm-cen-trunk-osx64/build/objdir/i386/mozilla/dist/include/nspr --cflags` -I/builds/slave/comm-cen-trunk-osx64/build/objdir/i386/mozilla/dist/include/nss      -fPIC  -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Werror=conversion-null -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -gdwarf-2 -isysroot /Developer/SDKs/MacOSX10.6.sdk -fno-exceptions -fno-strict-aliasing -fno-common -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -DNO_X11  -DNDEBUG -DTRIMMED -gdwarf-2 -O3 -fomit-frame-pointer   -DMOZILLA_CLIENT -include ../../../comm-config.h -MD -MF .deps/nsImportMail.pp /builds/slave/comm-cen-trunk-osx64/build/mailnews/import/src/nsImportMail.cpp
nsImportMailboxDescriptor.cpp
In file included from ../../../../../mailnews/base/src/nsMessengerOSXIntegration.mm:7:
In file included from ../../../mozilla/dist/include/nsMsgUtils.h:10:
In file included from ../../../mozilla/dist/include/nsStringGlue.h:15:
In file included from ../../../mozilla/dist/include/nsString.h:180:
In file included from ../../../mozilla/dist/include/nsPromiseFlatString.h:16:
../../../mozilla/dist/include/nsTPromiseFlatString.h:105:12: error: functional-style cast from 'const nsString' to 'nsPromiseFlatString' uses deleted function
    return nsTPromiseFlatString_CharT(string);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../../mozilla/dist/include/string-template-def-unichar.h:24:45: note: expanded from macro 'nsTPromiseFlatString_CharT'
#define nsTPromiseFlatString_CharT          nsPromiseFlatString
                                            ^
../../../../../mailnews/base/src/nsMessengerOSXIntegration.mm:645:31: note: in instantiation of function template specialization 'PromiseFlatString<nsString>' requested here
                              PromiseFlatString(author).get());
                              ^
../../../mozilla/dist/include/nsTPromiseFlatString.h:78:7: note: candidate constructor has been explicitly deleted
      nsTPromiseFlatString_CharT( const string_type& str ) MOZ_DELETE;
      ^
../../../mozilla/dist/include/string-template-def-unichar.h:24:45: note: expanded from macro 'nsTPromiseFlatString_CharT'
#define nsTPromiseFlatString_CharT          nsPromiseFlatString
                                            ^
../../../mozilla/dist/include/nsTPromiseFlatString.h:83:7: note: candidate constructor
      nsTPromiseFlatString_CharT( const substring_type& str )
      ^
../../../mozilla/dist/include/string-template-def-unichar.h:24:45: note: expanded from macro 'nsTPromiseFlatString_CharT'
#define nsTPromiseFlatString_CharT          nsPromiseFlatString
                                            ^
Blocks: 824424

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: