Closed Bug 1232732 Opened 7 years ago Closed 7 years ago

fix MOZ_WIN_MEM_TRY_CATCH to work correctly with clang-cl

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: froydnj, Unassigned)

References

Details

Attachments

(1 file)

MOZ_WIN_MEM_TRY_CATCH contains:

  NS_WARNING("EXCEPTION_IN_PAGE_ERROR in " __FUNCTION__);

Requiring string concatenation here is apparently an MSVC extension.  LLVM's bug database has a WONTFIX bug against this:

https://llvm.org/bugs/show_bug.cgi?id=20013

Even though recent comments in the bug suggest that LLVM would accept a patch for this behavior ("seems difficult"), we should probably fix our code.  Deleting this NS_WARNING seems reasonable; making it use nsPrintfCString also seems reasonable.

This is the source of two files falling back to MSVC when compiling with clang-cl.  Fixing this would mean that all first-party Gecko code would work without falling back to MSVC.
The particular syntax for this warning is an MSVC extension, which
causes clang-cl to fallback to MSVC when compiling files that use this
macro.  Trying to use nsPrintfCString or equivalent here runs into
issues of unused variables in non-debug builds.  I think it's reasonable
to raise an warning without a specific location; folks can grovel about
in a debugger if they want to figure out what's going on.
Attachment #8703752 - Flags: review?(aklotz)
Comment on attachment 8703752 [details] [diff] [review]
modify NS_WARNING in MOZ_WIN_MEM_TRY_CATCH

Review of attachment 8703752 [details] [diff] [review]:
-----------------------------------------------------------------

Seems reasonable. I suppose another option is to define the macro differently depending on whether clang-cl is present. Either way is fine with me.
Attachment #8703752 - Flags: review?(aklotz) → review+
https://hg.mozilla.org/mozilla-central/rev/c82bd0fbe6b6
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.