Closed Bug 1023449 Opened 6 years ago Closed 6 years ago

Try to check for MSVC before clang so that we live in the clang-cl world when building with that compiler

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file)

No description provided.
Assignee: nobody → ehsan
Blocks: winclang
Attachment #8437838 - Flags: review?(nfroyd)
Comment on attachment 8437838 [details] [diff] [review]
Try to check for MSVC before clang so that we live in the clang-cl world when building with that compiler

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

r=me with the change below.

::: mfbt/Attributes.h
@@ +53,5 @@
> +#  define MOZ_HAVE_CXX11_OVERRIDE
> +#  define MOZ_HAVE_NEVER_INLINE          __declspec(noinline)
> +#  define MOZ_HAVE_NORETURN              __declspec(noreturn)
> +// Staying away from explicit conversion operators in MSVC for now, see
> +// http://stackoverflow.com/questions/20498142/visual-studio-2013-explicit-keyword-bug

I think this text should be moved to...

@@ +61,5 @@
>   * detectable by checking whether __GXX_EXPERIMENTAL_CXX0X__ is defined or, more
>   * standardly, by checking whether __cplusplus has a C++11 or greater value.
>   * Current versions of g++ do not correctly set __cplusplus, so we check both
>   * for forward compatibility.
>   */

...inside this comment, probably with a revision to something like:

"Even though some versions of MSVC support explicit conversion operators, we don't indicate
support for them here, due to
http://stackoverflow.com/questions/20498142/visual-studio-2013-explicit-keyword-bug"

And then this entire comment should be moved above the #if defined(_MSC_VER) check.
Attachment #8437838 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/eaa1f380f560
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.