Skip to content

compiler: remove invalid pragmas from test fixture - #5894

Merged
deadprogram merged 1 commit into
tinygo-org:devfrom
davecheney:pragma-fixture-cleanup
Oct 19, 2026
Merged

deadprogram merged 1 commit into
tinygo-org:devfrom
davecheney:pragma-fixture-cleanup

Conversation

@davecheney

Copy link
Copy Markdown
Contributor

Both pragmas are ignored in these positions. Remove them before adding stricter pragma checks. The expected LLVM IR is unchanged.

Both pragmas are ignored in these positions. Remove them before adding
stricter pragma checks. The expected LLVM IR is unchanged.

// The //go:noescape pragma only works on declarations, not definitions.
//
//go:noescape

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm confused, why does this have to be removed? (Not that I know why this was there in the first place)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have a PR coming which will validate pragmas better; it started from the //go:nolinktime PR, but I spotted that other pragmas could validate their arguments sooner and spot invalid use rather than silently being ignored

@davecheney

Copy link
Copy Markdown
Contributor Author

@deadprogram PTAL, this is related to #5887 and is a prerequisite for adding better validation of pragmas. I'm doing the cleanups now because I don't want to mix the fixups with the compiler change and keep the change smaller.

@deadprogram deadprogram left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix @davecheney and to @jakebailey for review. Now merging.

@deadprogram
deadprogram merged commit 84e957c into tinygo-org:dev Oct 19, 2026
33 checks passed
@davecheney
davecheney deleted the pragma-fixture-cleanup branch October 19, 2026 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants