Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23064 )

Change subject: [Rocky] avoid -Wpsabi warning in ComputeLowerAndUpperBound
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/23064/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23064/5//COMMIT_MSG@25
PS5, Line 25: is low
That's not just low, that's non-existent because the function 
ComputeLowerAndUpperBound() isn't in the Kudu's ABI.  Moreover, Kudu RELEASE 
builds by default link all Kudu intermediate libraries directly into 
kudu-master, kudu-tserver, and kudu CLI binaries, so there isn't a chance to 
mix and match Kudu libraries compiled by different compilers.


http://gerrit.cloudera.org:8080/#/c/23064/5//COMMIT_MSG@26
PS5, Line 26: However, to silence the warning and avoid any future ambiguity, 
this
            : patch replaces the std::pair return type with a custom BoundRange 
struct.
If the main premise of this changelist is to avoid the warning from some 
particular compilers at a particular platform (at least per wording in the 
summary), why not just silence this particular warning with a targeted 
compiler's pragma instead of introducing any modifications?


http://gerrit.cloudera.org:8080/#/c/23064/5//COMMIT_MSG@29
PS5, Line 29: Older compilers already treated such a struct as an HFA, so this 
change
            : makes the ABI consistent across versions. With newer compilers, 
the
            : generated code is unchanged. With older compilers, the function 
may pass
            : the return value through a different register, that does not 
affect
            : correctness.
As already mentioned above, this isn't applicable since 
ComputeLowerAndUpperBound isn't a part of Kudu ABI, so why to fuss about this?

On the related note, this wouldn't be even a point to mention if simply 
suppressing the warning.



--
To view, visit http://gerrit.cloudera.org:8080/23064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab4db585a8eaf152f1fa2884e72e38a4e7c5efa1
Gerrit-Change-Number: 23064
Gerrit-PatchSet: 5
Gerrit-Owner: KeDeng <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Zoltan Martonka <[email protected]>
Gerrit-Comment-Date: Wed, 25 Jun 2025 21:43:12 +0000
Gerrit-HasComments: Yes

Reply via email to