Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-12325: [C++] [CI] Nightly gandiva build failing due to failure of compiler to move return value #9975

Closed
wants to merge 1 commit into from

Conversation

westonpace
Copy link
Member

@westonpace westonpace commented Apr 10, 2021

Gandiva build failure caused by failure of GCC 4.8.2 compiler to automatically move return value.

Failed build: https://github.com/ursacomputing/crossbow/runs/2303374510
Godbolt showing I'm not crazy: https://gcc.godbolt.org/z/x138zevhE

@github-actions
Copy link

@nealrichardson
Copy link
Member

I'm open to suggestions

Maybe add a comment to this effect so that whenever we remove support for gcc < 4.9, we can remove this? Or could you even use an #ifdef or something to only do this on gcc < 4.9?

@lidavidm
Copy link
Member

lidavidm commented Apr 14, 2021

I agree, an ifdef could be OK, though right now this is only used in tests.

This builds/passes tests: https://github.com/lidavidm/crossbow/actions/runs/748817979 (it fails on uploading but we don't care about that).

@westonpace
Copy link
Member Author

I'll side with @cyb70289 and @lidavidm that an ifdef would probably be overkill. Feel free to merge.

@kiszk
Copy link
Member

kiszk commented Apr 17, 2021

ifdef looks more clear to show our intention.

…urn local variables if the output type does not match exactly the return type.
@westonpace
Copy link
Member Author

@cyb70289 I updated line 95 and fixed up the commit message. @pitrou is correct that we do not need to worry about RVO if the output type differs so it should be harmless (i.e. no need for an ifdef).

@kou
Copy link
Member

kou commented Apr 21, 2021

Can we merge this to include this into 4.0.0 RC2?

@westonpace
Copy link
Member Author

The build failures appear to all be expected. MacOS is failing due to LLVM (ARROW-12467). C GLib & Ruby is failing on Windows with segmentation fault but was doing so before this (we should probably create a JIRA). The JNI failure is ARROW-11633

@cyb70289 cyb70289 closed this in 2ed54db Apr 21, 2021
GeorgeAp pushed a commit to sirensolutions/arrow that referenced this pull request Jun 7, 2021
…of compiler to move return value

Gandiva build failure caused by failure of GCC 4.8.2 compiler to automatically move return value.

Failed build: https://github.com/ursacomputing/crossbow/runs/2303374510
Godbolt showing I'm not crazy: https://gcc.godbolt.org/z/x138zevhE

Closes apache#9975 from westonpace/bugfix/ARROW-12325

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Yibo Cai <yibo.cai@arm.com>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 10, 2021
…of compiler to move return value

Gandiva build failure caused by failure of GCC 4.8.2 compiler to automatically move return value.

Failed build: https://github.com/ursacomputing/crossbow/runs/2303374510
Godbolt showing I'm not crazy: https://gcc.godbolt.org/z/x138zevhE

Closes apache#9975 from westonpace/bugfix/ARROW-12325

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Yibo Cai <yibo.cai@arm.com>
michalursa pushed a commit to michalursa/arrow that referenced this pull request Jun 13, 2021
…of compiler to move return value

Gandiva build failure caused by failure of GCC 4.8.2 compiler to automatically move return value.

Failed build: https://github.com/ursacomputing/crossbow/runs/2303374510
Godbolt showing I'm not crazy: https://gcc.godbolt.org/z/x138zevhE

Closes apache#9975 from westonpace/bugfix/ARROW-12325

Authored-by: Weston Pace <weston.pace@gmail.com>
Signed-off-by: Yibo Cai <yibo.cai@arm.com>
@westonpace westonpace deleted the bugfix/ARROW-12325 branch January 6, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants