GH-148047: Check early whether tail-calling is possible for MSVC builds on Windows#148036
GH-148047: Check early whether tail-calling is possible for MSVC builds on Windows#148036chris-eibl wants to merge 4 commits intopython:mainfrom
Conversation
|
TSan https://github.com/python/cpython/actions/runs/23948631142/job/69850491565?pr=148036 failure for sure unreleated: this is a pure Windows change and would result in a build error on onsupported (Windows) hosts when using I've checked manually that I get all the expected errors for unsupported configurations. |
|
Please open a separate issue for this, rather than reusing the same closed issue. Thanks! |
chris-eibl
changed the title
chris-eibl
added
OS-windows
build
chris-eibl
changed the title
itamaro
left a comment
itamaro
left a comment
There was a problem hiding this comment.
Overall looks good, thank you for this!
| <Delete Files="$(OutDir)pybuilddir.txt" /> | ||
| </Target> | ||
|
|
||
| <Target Name="_CheckTailCalling" BeforeTargets="PrepareForBuild" Condition="'$(UseTailCallInterp)' == 'true' and $(PlatformToolset) != 'ClangCL'"> |
There was a problem hiding this comment.
Should the condition match on "is msvc" instead of "not clangcl"?
| <Error Text="Platform toolset >= v145 is required for tail-calling." | ||
| Condition="$(PlatformToolset.Replace('v', '0')) < '145'" /> | ||
| <Error Text="MSVC requires optimization to be enabled for tail-calling." | ||
| Condition="$(Configuration) == 'Debug'" /> |
There was a problem hiding this comment.
I don't remember all the possible knobs and switches to configure the Windows build -- is 'config is Debug' the canonical and reliable way to determine "optimizations not enabled"?
Is it possible, using the correct incantation, to enable optimizations while keeping the config as "Debug"? or have non-Debug config while disabling optimizations?
| @@ -0,0 +1,2 @@ | |||
| Check early whether tail-calling is possible for MSVC builds on Windows. | |||
There was a problem hiding this comment.
Maybe make this more about "fail fast with an explicit and clear error message" (which is the primary user-facing benefit here)
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
3 participants
Rather than failing late when compiling e.g. a debug configuration
with hundreds of
lets fail early with an explicit error message for configurations that are not supported by MSVC.
This is a follow-up on #140513 / #140548.