On Fri, 30 Aug 2024 22:03:11 GMT, Phil Race <p...@openjdk.org> wrote:

>> I saw following errors (warnings) when I tried to build OpenJDK on Fedora 40 
>> with gcc-14.2.1-1.fc40.x86_64:
>> 
>> 
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libfontmanager/HBShaper_Panama.c:
>>  In function ‘jdk_hb_shape’:
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libfontmanager/HBShaper_Panama.c:101:16:
>>  error: unused variable ‘yPtSize’ [-Werror=unused-variable]
>>   101 | float yPtSize = euclidianDistance(matrix[2], matrix[3]);
>>       | ^~~~~~~
>> cc1: all warnings being treated as errors
>> * For target support_native_java.desktop_libfontmanager_X11FontScaler.o:
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/unix/native/libfontmanager/X11FontScaler.c:
>>  In function ‘Java_sun_font_NativeFont_getGlyphAdvance’:
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/unix/native/libfontmanager/X11FontScaler.c:270:32:
>>  error: unused variable ‘descent’ [-Werror=unused-variable]
>>   270 | int direction, ascent, descent;
>>       | ^~~~~~~
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/unix/native/libfontmanager/X11FontScaler.c:270:24:
>>  error: unused variable ‘ascent’ [-Werror=unused-variable]
>>   270 | int direction, ascent, descent;
>>       | ^~~~~~
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/unix/native/libfontmanager/X11FontScaler.c:270:13:
>>  error: unused variable ‘direction’ [-Werror=unused-variable]
>>   270 | int direction, ascent, descent;
>>       | ^~~~~~~~~
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c:
>>  In function ‘Java_sun_font_FreetypeFontScaler_createScalerContextNative’:
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c:516:19:
>>  error: unused variable ‘scalerInfo’ [-Werror=unused-variable]
>>   516 | FTScalerInfo *scalerInfo =
>>       | ^~~~~~~~~~
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c:
>>  In function ‘Java_sun_font_FreetypeFontScaler_getGlyphPointNative’:
>> /home/ysuenaga/github-forked/jdk/src/java.desktop/share/native/libfontmanager/freetypeScaler.c:1655:13:
>>  error: unused variable ‘point’ [-Werror=unused-variable]
>>  1655 | jobject point = NULL;
>>       | ^~~~~
>
> src/java.desktop/share/native/libfontmanager/HBShaper_Panama.c line 101:
> 
>> 99:      if (getenv("HB_NODEVTX") != NULL) {
>> 100:          float xPtSize = euclidianDistance(matrix[0], matrix[1]);
>> 101:          float yPtSize = euclidianDistance(matrix[2], matrix[3]);
> 
> I paused to think about whether the problem is really that  the division 
> below should be using yPtSize .. in other words, the correct fix isn't always 
> as obvious as it might look.
> 
> For the record, I think the unused var is becuase it was patterned after the 
> code code in HBShaper.c which stores the yPtSize in a struct, but here it is 
> clear that it isn't used.
> 
>     fi->xPtSize = euclidianDistance(fi->matrix[0], fi->matrix[1]);
>     fi->yPtSize = euclidianDistance(fi->matrix[2], fi->matrix[3]);
>     if (getenv("HB_NODEVTX") != NULL) {
>         fi->devScale = fi->xPtSize / fi->ptSize;
> 
> 
> Now, as far as I can they aren't used except in that fn, so probably can also 
> go ..

In addition, I've confirmed `euclidianDistance` does not make any side-effects.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20790#discussion_r1739601532

Reply via email to