Summary: | [Win] Assertion fails when running JSC stress tests. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | peavo | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, bfulgham, commit-queue, darin, fpizlo, mark.lam | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
peavo
2014-08-20 08:39:59 PDT
Created attachment 236873 [details]
Patch
Comment on attachment 236873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236873&action=review > Source/JavaScriptCore/bytecode/CodeOrigin.h:143 > + static CodeSpecializationKind specializationKindFor(unsigned kind) This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? > Source/JavaScriptCore/bytecode/CodeOrigin.h:163 > + unsigned kind : 2; I believe our WebKit coding style pattern in cases like this is to put a comment off to the right with the name of the enum type. See examples like the ones in DFGNode.h, RenderObject.h, StyleRareInheritedData.h, and StyleRareNonInheritedData.h. Created attachment 236874 [details]
Patch
Comment on attachment 236874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review > Source/JavaScriptCore/bytecode/CodeOrigin.h:143 > - static CodeSpecializationKind specializationKindFor(Kind kind) > + static CodeSpecializationKind specializationKindFor(unsigned kind) This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? Comment on attachment 236874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review >> Source/JavaScriptCore/bytecode/CodeOrigin.h:143 >> + static CodeSpecializationKind specializationKindFor(unsigned kind) > > This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? There is likely a better way to fix this than to remove the type here. (In reply to comment #5) > (From update of attachment 236874 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review > > >> Source/JavaScriptCore/bytecode/CodeOrigin.h:143 > >> + static CodeSpecializationKind specializationKindFor(unsigned kind) > > > > This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? > > There is likely a better way to fix this than to remove the type here. MSVC was not able to convert from unsigned to the Kind enum type. Maybe replace with a static_cast when calling the function instead? Comment on attachment 236874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review >>>> Source/JavaScriptCore/bytecode/CodeOrigin.h:143 >>>> + static CodeSpecializationKind specializationKindFor(unsigned kind) >>> >>> This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? >> >> There is likely a better way to fix this than to remove the type here. > > MSVC was not able to convert from unsigned to the Kind enum type. > Maybe replace with a static_cast when calling the function instead? I see that we’re now using enum classes everywhere. How about using “enum class Kind : unsigned”? Comment on attachment 236874 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review >>>>> Source/JavaScriptCore/bytecode/CodeOrigin.h:143 >>>>> + static CodeSpecializationKind specializationKindFor(unsigned kind) >>>> >>>> This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? >>> >>> There is likely a better way to fix this than to remove the type here. >> >> MSVC was not able to convert from unsigned to the Kind enum type. >> Maybe replace with a static_cast when calling the function instead? > > I see that we’re now using enum classes everywhere. How about using “enum class Kind : unsigned”? Yes, we should put the static_cast into the specializationKind() function; note that the two other callers in DFGByteCodeParser.cpp would not require a static_cast. Also seems fine to try Mark’s suggestion of moving to an enum class, but I think that will lead to even more changes and I am not sure it’s worth it. Created attachment 236876 [details]
Patch
(In reply to comment #8) > (From update of attachment 236874 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review > > >>>>> Source/JavaScriptCore/bytecode/CodeOrigin.h:143 > >>>>> + static CodeSpecializationKind specializationKindFor(unsigned kind) > >>>> > >>>> This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? > >>> > >>> There is likely a better way to fix this than to remove the type here. > >> > >> MSVC was not able to convert from unsigned to the Kind enum type. > >> Maybe replace with a static_cast when calling the function instead? > > > > I see that we’re now using enum classes everywhere. How about using “enum class Kind : unsigned”? > > Yes, we should put the static_cast into the specializationKind() function; note that the two other callers in DFGByteCodeParser.cpp would not require a static_cast. Also seems fine to try Mark’s suggestion of moving to an enum class, but I think that will lead to even more changes and I am not sure it’s worth it. Ok, thanks, updated patch :) (In reply to comment #7) > (From update of attachment 236874 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=236874&action=review > > >>>> Source/JavaScriptCore/bytecode/CodeOrigin.h:143 > >>>> + static CodeSpecializationKind specializationKindFor(unsigned kind) > >>> > >>> This change should not be necessary. What happens when you don’t change this? Some kind of failure to compile? > >> > >> There is likely a better way to fix this than to remove the type here. > > > > MSVC was not able to convert from unsigned to the Kind enum type. > > Maybe replace with a static_cast when calling the function instead? > > I see that we’re now using enum classes everywhere. How about using “enum class Kind : unsigned”? Yes. See <https://bugs.webkit.org/show_bug.cgi?id=134252> as well. We've hit this numerous times. Comment on attachment 236876 [details] Patch Clearing flags on attachment: 236876 Committed r172801: <http://trac.webkit.org/changeset/172801> All reviewed patches have been landed. Closing bug. |