GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-132132: Upgrade LLVM on tail calling CI by Fidget-Spinner · Pull Request #132098 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented Apr 4, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Fidget-Spinner
Fix double redeclaration on MSVC+clang-cl
701aa8d
@bedevere-app bedevere-app Bot added the awaiting core review label Apr 4, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Apr 4, 2025
Implement PEP 768 – Safe external debugger interface for CPython #131591
Closed
@Fidget-Spinner

Fidget-Spinner commented Apr 4, 2025

Copy link
Copy Markdown
Member Author

@chris-eibl can I take some of your time to request a review please :)?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fidget-Spinner
trigger CI
e449148
@Fidget-Spinner
Fidget-Spinner requested review from AA-Turner, ezio-melotti and hugovk as code owners April 4, 2025 17:56
@Fidget-Spinner Fidget-Spinner added the skip news label Apr 4, 2025
Fidget-Spinner added 4 commits April 5, 2025 02:05
@Fidget-Spinner
Upgrade to LLVM 20
5eea591
@Fidget-Spinner
Fix
fcb45ef
@Fidget-Spinner
fix macos
ce1a4ae
@Fidget-Spinner
fix macOS
d2002cd
@Fidget-Spinner
Fidget-Spinner requested a review from ericsnowcurrently as a code owner April 4, 2025 18:26
@Fidget-Spinner Fidget-Spinner changed the title gh-131591: Fix double redeclaration on MSVC+clang-cl gh-131591: Upgrade LLVM on tail calling, and fix MSVC Apr 4, 2025
Fidget-Spinner added 4 commits April 5, 2025 02:30
@Fidget-Spinner
fix macOS for real this time
c8afa07
@Fidget-Spinner
maybe fix macOS?
728eb68
@Fidget-Spinner
Convince clang to behave like on Linux
bc1d927
@Fidget-Spinner
remove useless extern
a468604
@chris-eibl

chris-eibl commented Apr 4, 2025

Copy link
Copy Markdown
Member

Oh no, it's back again - this was already a back and forth (#130040).

I think, your fix back then (exclude __clang__)

#if defined(MS_WINDOWS) && !defined(__clang__)
#define _GENERATE_DEBUG_SECTION_WINDOWS(name)                       \
   _Pragma(Py_STRINGIFY(section(Py_STRINGIFY(name), read, write))) \
   __declspec(allocate(Py_STRINGIFY(name)))

should stay: clangcl might not understand __declspec(allocate(...)).

But then, no GENERATE_DEBUG_SECTION is in effect, because

#if defined(__linux__) && (defined(__GNUC__) || defined(__clang__))
#define _GENERATE_DEBUG_SECTION_LINUX(name) \
   __attribute__((section("." Py_STRINGIFY(name))))               \
   __attribute__((used))

will never fire due to __linux__. Maybe rewrite that to

#if (defined(__linux__) && defined(__GNUC__)) || (defined(MS_WINDOWS) && defined(__clang__))
#define _GENERATE_DEBUG_SECTION_LINUX(name) \
   __attribute__((section("." Py_STRINGIFY(name))))               \
   __attribute__((used))

clang defines __GNUC__, so I dropped the first __clang__

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl

chris-eibl commented Apr 4, 2025

Copy link
Copy Markdown
Member

Ah - while I was typing you already had the same idea :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner added 2 commits April 5, 2025 03:05
@Fidget-Spinner
Update remote_debugging.c
07cafa9
@Fidget-Spinner
Update remote_debugging.c
bd00b78
chris-eibl
chris-eibl reviewed Apr 4, 2025
View reviewed changes
Comment thread Python/remote_debugging.c Outdated
const char* secname = "_PyRuntime";
#else
const char* secname = "PyRuntime";
#endif

@chris-eibl chris-eibl Apr 4, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use secname instead of "PyRuntime" below?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@Fidget-Spinner Fidget-Spinner Apr 4, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah oops 🤦 thanks 3AM things.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chris-eibl reacted with laugh emoji
All reactions
  • 😄 1 reaction
@Fidget-Spinner
Update remote_debugging.c
04d34df
taegyunkim
taegyunkim reviewed Apr 4, 2025
View reviewed changes
Comment thread .github/workflows/tail-call.yml
Comment on lines +117 to +118
export PATH="/usr/local/opt/llvm/bin:$PATH"
export PATH="/opt/homebrew/opt/llvm/bin:$PATH"

@taegyunkim taegyunkim Apr 4, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonder whether
eval "$(brew shellenv)"
is just enough for both intel and arm mac

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@Fidget-Spinner Fidget-Spinner Apr 4, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. Let me try that later.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@Fidget-Spinner

Fidget-Spinner commented Apr 4, 2025

Copy link
Copy Markdown
Member Author

Well I have absolutely no clue what's going on with Windows here. Will sleep on it and let someone else take over. In the meantime, will just use this PR to upgrade the LLVM version.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

chris-eibl
chris-eibl reviewed Apr 4, 2025
View reviewed changes
Comment thread Python/remote_debugging.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl chris-eibl mentioned this pull request Apr 5, 2025
gh-131591: Fix GENERATE_DEBUG_SECTION for clangcl on Windows #132112
Merged
@chris-eibl

chris-eibl commented Apr 5, 2025

Copy link
Copy Markdown
Member

@Fidget-Spinner: I've created #132112 to fix GENERATE_DEBUG_SECTION for clangcl on Windows. CI is green. Feel free to incorporate it in this PR if you want - I'll then just close it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Fidget-Spinner added 2 commits April 6, 2025 00:03
@Fidget-Spinner
revert debug offset changes
5de6b43
@Fidget-Spinner
Merge remote-tracking branch 'upstream/main' into fix-double-redeclar…
e786fd8
…ation
@Fidget-Spinner Fidget-Spinner changed the title gh-131591: Upgrade LLVM on tail calling, and fix MSVC Upgrade LLVM on tail calling CI Apr 5, 2025
@Fidget-Spinner Fidget-Spinner added the skip issue label Apr 5, 2025
@Fidget-Spinner
try eval brew shellenv
e4ff0e0
@Fidget-Spinner Fidget-Spinner changed the title Upgrade LLVM on tail calling CI gh-132132: Upgrade LLVM on tail calling CI Apr 5, 2025
@Fidget-Spinner
revert eval
b9c97f6
@Fidget-Spinner

Fidget-Spinner commented Apr 5, 2025

Copy link
Copy Markdown
Member Author

@chris-eibl can you please send me an email (it's listed on my GH profile), I want to ask some questions.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@chris-eibl

chris-eibl commented Apr 5, 2025

Copy link
Copy Markdown
Member

Email sent :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Fidget-Spinner
Fidget-Spinner merged commit 92fb949 into python:main Apr 5, 2025
@bedevere-app bedevere-app Bot removed the awaiting core review label Apr 5, 2025
@Fidget-Spinner
Fidget-Spinner deleted the fix-double-redeclaration branch April 5, 2025 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chris-eibl chris-eibl chris-eibl left review comments
@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner
@hugovk hugovk Awaiting requested review from hugovk hugovk is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently
+1 more reviewer
@taegyunkim taegyunkim taegyunkim left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@Fidget-Spinner @chris-eibl @taegyunkim
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.