CSS3 calc() - simple parse time evaluation
Created attachment 125806 [details] Patch
Comment on attachment 125806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125806&action=review I know it's a pain to post these smaller patches, but these are 1000x easier to review. :) > Source/WebCore/css/CSSCalculationValue.cpp:117 > + > + ASSERT_NOT_REACHED(); > + return 0; Does the compiler complain if you leave this out? If not, I'd just delete this since you will get a warning if a new Calc type is added since your switch above doesn't have a "default" clause (== a good thing!). > Source/WebCore/css/CSSCalculationValue.cpp:206 > + case CalcMod: > + if (rightValue) > + return static_cast<int>(leftValue) % static_cast<int>(rightValue); > + return std::numeric_limits<double>::quiet_NaN(); Wasn't mod dropped from the spec? > Source/WebCore/css/CSSCalculationValue.cpp:209 > + ASSERT_NOT_REACHED(); > + return 0; Ditto re: deleting the unreachable code if the compiler doesn't complain.
Comment on attachment 125806 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125806&action=review >> Source/WebCore/css/CSSCalculationValue.cpp:117 >> + return 0; > > Does the compiler complain if you leave this out? If not, I'd just delete this since you will get a warning if a new Calc type is added since your switch above doesn't have a "default" clause (== a good thing!). Nope, it doesn't. Removed. >> Source/WebCore/css/CSSCalculationValue.cpp:206 >> + return std::numeric_limits<double>::quiet_NaN(); > > Wasn't mod dropped from the spec? Indeed it has been. I've removed the evaluation code, and I'll get rid of the enum value in another patch. >> Source/WebCore/css/CSSCalculationValue.cpp:209 >> + return 0; > > Ditto re: deleting the unreachable code if the compiler doesn't complain. Removed.
Created attachment 125956 [details] Patch for landing
Comment on attachment 125956 [details] Patch for landing Clearing flags on attachment: 125956 Committed r107030: <http://trac.webkit.org/changeset/107030>
All reviewed patches have been landed. Closing bug.