Closed
Bug 1228482
Opened 5 years ago
Closed 5 years ago
nsEdgeReadingListExtractor.cpp(205): error C2665 with VS2015
Categories
(Firefox :: Migration, defect)
Firefox
Migration
Tracking
()
RESOLVED
FIXED
Firefox 45
People
(Reporter: xidorn, Assigned: glandium)
References
Details
Attachments
(1 file)
0:49.92 central/browser/components/migration/nsEdgeReadingListExtractor.cpp(205): error C2665: 'swprintf': none of the 2 overloads could convert all the argument types 0:49.92 C:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt\corecrt_wstdio.h(1820): note: could be 'int swprintf(wchar_t *const ,const wchar_t *const ,...) throw()' 0:49.92 C:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt\corecrt_wstdio.h(1463): note: or 'int swprintf(wchar_t *const ,const size_t,const wchar_t *const ,...)' 0:49.92 central/browser/components/migration/nsEdgeReadingListExtractor.cpp(205): note: while trying to match the argument list '(wchar_t *, int, const char16_t [44], const JET_ERR)' 0:49.92 central/browser/components/migration/nsEdgeReadingListExtractor.cpp(206): error C2664: 'nsresult nsIConsoleService::LogStringMessage(const char16_t *)': cannot convert argument 1 from 'wchar_t *' to 'const char16_t *' 0:49.92 central/browser/components/migration/nsEdgeReadingListExtractor.cpp(206): note: Types pointed to are unrelated; conversion requires reinterpret_cast, C-style cast or function-style cast
Reporter | ||
Comment 1•5 years ago
|
||
Bug 1228482 - Fix compile error with VS2015 and memory leak in nsEdgeReadingListExtractor.
Attachment #8692785 -
Flags: review?(jmathies)
Comment 2•5 years ago
|
||
Ugh. This is the third time I've done this now. When I wrote the patch in bug 1225798, I explicitly went looking for what the previous adjustments had done to make sure that what I would be doing would not cause the same kind of breakage again (bug 1213246, bug 1200187). I was under the impression that MOZ_UTF16 was supposed to get around this issue (that was certainly how some of the other breakage was fixed). I couldn't find any information on MDN, and did find out that we do our own wrapping of some of the wide string types. For someone not familiar with this and not in possession of msvc2015 this seems impossible to get right. Can you (or someone else who knows) please write a document on MDN with details about how to deal with this kind of problem correctly?
Reporter | ||
Comment 3•5 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #2) > I was under the impression > that MOZ_UTF16 was supposed to get around this issue (that was certainly how > some of the other breakage was fixed). MOZ_UTF16 is not. MOZ_UTF16("string") is simply extended to L"string" (on VS2013) and u"string" (otherwise). The former has type "const wchar_t[]" and the latter is "const char16_t[]". On VS2013, there is no native char16_t, so we alias char16_t to wchar_t, however on VS2015, those two become two independent types. > I couldn't find any information on > MDN, and did find out that we do our own wrapping of some of the wide string > types. For someone not familiar with this and not in possession of msvc2015 > this seems impossible to get right. I totally agree. I could get it wrong as well if I do not build on VS2015. > Can you (or someone else who knows) please write a document on MDN with > details about how to deal with this kind of problem correctly? I guess I can... Leave a ni? for myself.
Flags: needinfo?(quanxunzhen)
Assignee | ||
Comment 4•5 years ago
|
||
You should just avoid swprintf and wchar_t altogether. It's not like we have an entire String API in xpcom/ to do string formatting.
Comment 5•5 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #4) > You should just avoid swprintf and wchar_t altogether. It's not like we have > an entire String API in xpcom/ to do string formatting. ... and yet the console service's API takes a wstring, and has no API that takes an XPCOM string. It has one that takes an nsIConsoleMessage, but that's harder to construct and overkill for what is going on here.
Comment 6•5 years ago
|
||
Comment on attachment 8692785 [details] MozReview Request: Bug 1228482 - Fix compile error with VS2015 and memory leak in nsEdgeReadingListExtractor. https://reviewboard.mozilla.org/r/26323/#review23779 Looks good to me.
Attachment #8692785 -
Flags: review+
Assignee | ||
Comment 8•5 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #5) > (In reply to Mike Hommey [:glandium] from comment #4) > > You should just avoid swprintf and wchar_t altogether. It's not like we have > > an entire String API in xpcom/ to do string formatting. > > ... and yet the console service's API takes a wstring, and has no API that > takes an XPCOM string. It has one that takes an nsIConsoleMessage, but > that's harder to construct and overkill for what is going on here. a wstring is a char16_t.
Assignee | ||
Updated•5 years ago
|
Keywords: leave-open
Reporter | ||
Comment 10•5 years ago
|
||
Well, I guess instead of adding some document to describe this, it is probably better to just break compiling on VS2013 if someone tries to do implicit conversion between wchar_t and char16_t. That says, we should not typedef wchar_t to char16_t on VS2013, but instead, declaring an independent class to simulate all its behaviors except the implicit conversion from/to wchar_t.
Flags: needinfo?(quanxunzhen)
Assignee | ||
Comment 11•5 years ago
|
||
The best thing to do is to avoid wchar_t unless you're _very_ sure there's no other way than to use it. IOW, unless Windows API that require it are involved, you shouldn't be using it. And you should avoid system string functions that require it (like swprintf) and use our own String APIs instead. That's the rule of thumb.
Reporter | ||
Updated•5 years ago
|
Attachment #8692785 -
Flags: review?(jmathies)
Comment 12•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4f04c4b2cf60 https://hg.mozilla.org/mozilla-central/rev/b7675a900e87
Reporter | ||
Updated•5 years ago
|
Assignee: nobody → mh+mozilla
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
You need to log in
before you can comment on or make changes to this bug.
Description
•