Bug 26577 - Same warnings in yarr
Summary: Same warnings in yarr
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-20 17:56 PDT by Laszlo Gombos
Modified: 2010-06-12 15:18 PDT (History)
2 users (show)

See Also:


Attachments
Kill some warnings. (2.91 KB, patch)
2009-06-20 18:01 PDT, Laszlo Gombos
darin: review-
Details | Formatted Diff | Diff
Revised patch based on Darin's feedback. (2.93 KB, patch)
2009-06-21 15:21 PDT, Laszlo Gombos
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-06-20 17:56:31 PDT
Fix the following warnings on Linux/GCC 4.2.2 

yarr/RegexCompiler.cpp: In member function 'void JSC::Yarr::CharacterClassConstructor::putRange(UChar, UChar)':
yarr/RegexCompiler.cpp:126: warning: comparison between signed and unsigned integer expressions
yarr/RegexCompiler.cpp:132: warning: comparison between signed and unsigned integer expressions
yarr/RegexInterpreter.cpp: In member function 'bool JSC::Yarr::Interpreter::tryConsumeCharacter(int)':
yarr/RegexInterpreter.cpp:290: warning: comparison between signed and unsigned integer expressions
yarr/RegexInterpreter.cpp:290: warning: comparison between signed and unsigned integer expressions
Comment 1 Laszlo Gombos 2009-06-20 18:01:20 PDT
Created attachment 31603 [details]
Kill some warnings.
Comment 2 Darin Adler 2009-06-20 23:29:02 PDT
Comment on attachment 31603 [details]
Kill some warnings.

Getting rid of these warnings seems good. However, adding C-style typecasts is not so good.

> -                        while ((++unicodeCurr <= hi) && isUnicodeUpper(unicodeCurr) && (Unicode::toLower(unicodeCurr) == (lowerCaseRangeEnd + 1)))
> +                        while ((++unicodeCurr <= hi) && isUnicodeUpper(unicodeCurr) && (Unicode::toLower(unicodeCurr) == (UChar)(lowerCaseRangeEnd + 1)))

Maybe we can avoid the warning here by changing the 1 to a 1U rather than casting to UChar. Would you try that?

> -        if (pattern->m_ignoreCase ? ((Unicode::toLower(testChar) == ch) || (Unicode::toUpper(testChar) == ch)) : (testChar == ch)) {
> +        if (pattern->m_ignoreCase ? ((Unicode::toLower(testChar) == (UChar32)ch) || (Unicode::toUpper(testChar) == (UChar32)ch)) : (testChar == ch)) {

In this case I suggest we change the type of the "ch" local variable to UChar32 from int rather than adding the typecasts. Could you try that?

review- because I'd prefer a version that doesn't add typecasts. If you find my suggestions don't work feel free to put the original patch up for review again, but please use static_cast rather than C-style casts.
Comment 3 Laszlo Gombos 2009-06-21 15:21:42 PDT
Created attachment 31621 [details]
Revised patch based on Darin's feedback.

I did noticed that JSC::Yarr::Interpreter::tryConsumeCharacter() does not seems to be used. I was not brave enough to remove it but maybe the function should go instead of fixing it.
Comment 4 Darin Adler 2009-06-21 18:04:20 PDT
Comment on attachment 31621 [details]
Revised patch based on Darin's feedback.

r=me
Comment 5 Eric Seidel (no email) 2009-06-24 00:11:43 PDT
Comment on attachment 31621 [details]
Revised patch based on Darin's feedback.

Compile fails:

    /Developer/usr/bin/distcc /Developer/usr/bin/gcc-4.2 -x c++ -arch i386 -fmessage-length=0 -pipe -Wno-trigraphs -fno-exceptions -fno-rtti -fpascal-strings -fasm-blocks -O0 -Werror -Wnon-virtual-dtor -Wnewline-eof -DHAVE_DTRACE=0 -DWEBKIT_VERSION_MIN_REQUIRED=WEBKIT_VERSION_LATEST -fstrict-aliasing -fvisibility-inlines-hidden -fno-threadsafe-statics -mmacosx-version-min=10.5 -gdwarf-2 -I/Users/eseidel/Projects/build/JavaScriptCore.build/Debug/JavaScriptCore.build/JavaScriptCore.hmap -Wall -Wextra -Wcast-align -Wcast-qual -Wchar-subscripts -Wextra-tokens -Wformat=2 -Winit-self -Wmissing-format-attribute -Wmissing-noreturn -Wpacked -Wpointer-arith -Wredundant-decls -Wundef -Wwrite-strings -Wshorten-64-to-32 -fpch-preprocess -F/Users/eseidel/Projects/build/Debug -I/Users/eseidel/Projects/build/Debug/include -I/Users/eseidel/Projects/build/Debug/DerivedSources/JavaScriptCore -I. -Iicu -I/Users/eseidel/Projects/build/JavaScriptCore.build/Debug/JavaScriptCore.build/DerivedSources -include /var/folders/zz/zzzivhrRnAmviuee++2Pvk+-4yw/-Caches-/com.apple.Xcode.72687/SharedPrecompiledHeaders/JavaScriptCorePrefix-ddaljcarndktnmciaabrbfjegeul/JavaScriptCorePrefix.h -c /Users/eseidel/Projects/WebKit/JavaScriptCore/yarr/RegexCompiler.cpp -o /Users/eseidel/Projects/build/JavaScriptCore.build/Debug/JavaScriptCore.build/Objects-normal/i386/RegexCompiler.o
cc1plus: warnings being treated as errors
/Users/eseidel/Projects/WebKit/JavaScriptCore/yarr/RegexCompiler.cpp: In member function 'void JSC::Yarr::CharacterClassConstructor::putRange(UChar, UChar)':
/Users/eseidel/Projects/WebKit/JavaScriptCore/yarr/RegexCompiler.cpp:126: warning: comparison between signed and unsigned integer expressions
/Users/eseidel/Projects/WebKit/JavaScriptCore/yarr/RegexCompiler.cpp:132: warning: comparison between signed and unsigned integer expressions
distcc[38793] ERROR: compile /Users/eseidel/Projects/WebKit/JavaScriptCore/yarr/RegexCompiler.cpp on localhost failed
Comment 6 Alexey Proskuryakov 2010-06-12 15:18:24 PDT
These build problems have presumably been fixed over the year, closing. Please feel free to re-open if this is still a problem.