Skip to content

Conversation

m4rch3n1ng
Copy link
Contributor

@m4rch3n1ng m4rch3n1ng commented Feb 28, 2025

fixes: #250

#288 also partially addresses this, specifically only the parsing of unsafe extern "C" {} blocks, but it does so by adding them to the conflicts. this changes the implementation, so that you don't actually need the conflict field.

additionally this also parses safe fn and safe static, as well as unsafe static.

specified here, here and here

regarding #229:

diff
--- .tmp/list.txt	2025-02-28 16:16:12.002933806 +0100
+++ .tmp/list.txt.unsafe-extern-mod	2025-02-28 19:10:18.478498116 +0100
@@ -1,7 +1,6 @@
 tests/codegen/dst-offset.rs
 tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-user-defined-types.rs
 tests/codegen/function-arguments.rs
-tests/codegen/terminating-catchpad.rs
 tests/coverage/attr/nested.rs
 tests/coverage/coroutine.rs
 tests/coverage/coverage_attr_closure.rs
@@ -216,7 +215,6 @@
 tests/ui/parser/keyword-union-as-identifier.rs
 tests/ui/parser/parser-unicode-whitespace.rs
 tests/ui/parser/trait-plusequal-splitting.rs
-tests/ui/parser/unsafe-foreign-mod.rs
 tests/ui/pattern/issue-22546.rs
 tests/ui/pattern/usefulness/integer-ranges/issue-117648-overlapping_range_endpoints-false-positive.rs
 tests/ui/privacy/decl-macro-infinite-global-import-cycle-ice-64784.rs
@@ -230,8 +228,6 @@
 tests/ui/rust-2018/try-macro.rs
 tests/ui/rust-2018/uniform-paths/from-decl-macro.rs
 tests/ui/rust-2018/uniform-paths/macro-rules.rs
-tests/ui/rust-2024/unsafe-extern-blocks/extern-items.rs
-tests/ui/rust-2024/unsafe-extern-blocks/safe-items.rs
 tests/ui/simd/masked-load-store.rs
 tests/ui/sized/coinductive-1-gat.rs
 tests/ui/specialization/assoc-ty-graph-cycle.rs

@m4rch3n1ng m4rch3n1ng force-pushed the unsafe-extern-mod branch 6 times, most recently from a29fc35 to 8139f82 Compare March 7, 2025 01:08
@maxbrunsfeld
Copy link
Contributor

If possible, it would be cool to avoid the conflict by making the structure of function items and external blocks (and maybe other types of items) more consistent with each other. Generally, I'm ok with parsing a superset of what is actually allowed, if it gets us more consistency. Do you think there is any unification that makes sense between functions' modifiers, and extern blocks modifiers, and possibly any other items' modifiers?

@m4rch3n1ng
Copy link
Contributor Author

m4rch3n1ng commented Apr 1, 2025

last time i checked i thought this would be almost (if not outright) impossible, but thanks to your pointer (and actually learning more about tree-sitter over the last like month) i actually managed to do it without the conflict! very satisfying, though you can now write a const unsafe extern "C", which should be fine(?)

now just one more rebase ...

@m4rch3n1ng m4rch3n1ng force-pushed the unsafe-extern-mod branch 3 times, most recently from f0b18ce to 1b5d5e9 Compare April 1, 2025 22:01
@maxbrunsfeld
Copy link
Contributor

though you can now write a const unsafe extern "C", which should be fine(?)

Yeah, I'm generally ok with stuff like that. Seems like arguably a semantic error, as opposed to a syntactic one.

Just curious - how is the parser table size (STATE_COUNT) with this approach vs with the conflict?

@m4rch3n1ng
Copy link
Contributor Author

with this version it's 3861, with the conflict it's 3856. on master it's 3823.

@maxbrunsfeld
Copy link
Contributor

Ok, pretty small delta either way.

@m4rch3n1ng
Copy link
Contributor Author

m4rch3n1ng commented Apr 1, 2025

ci still doesn't work :(
(though it works for me locally?)

@maxbrunsfeld
Copy link
Contributor

Are you generating it with the latest tree-sitter-cli?

@m4rch3n1ng
Copy link
Contributor Author

i think so?

$ tree-sitter -V
tree-sitter 0.25.3 (2a835ee029dca1c325e6f1c01dbce40396f6123e)

@m4rch3n1ng
Copy link
Contributor Author

ok when i do a fresh clone of the repo, run npm install and then node --test bindings/node/*_test.js it doesn't work for me on master (commit 3691201) either. if i go back to the last commit before changing the abi to version 15 (3d087c3) that does work for me. i have no idea what is going on and i do not know how to fix it though sorry.

@m4rch3n1ng
Copy link
Contributor Author

m4rch3n1ng commented Apr 2, 2025

if i do tree-sitter init and just default everything, then do tree-sitter generate, npm install (and npm add tree-sitter as otherwise i get a module tree-sitter not found error) and then do npm test the exact same happens:

image
image

though i am not sure where i should report that to exactly.

@wetneb
Copy link

wetneb commented Jun 4, 2025

This PR is also integrated in the tree-sitter-rust-orchard fork, which has been set up so that generated files are not tracked in Git (motivated by the CI failures seen here). See #271 (comment) for context.

@m4rch3n1ng
Copy link
Contributor Author

apparently (for some reason) #288 also partially addressed this (the parsing of unsafe extern blocks), should i just fix the parsing of safe fn in this? or should i change the parsing implementation, as #288 fixes this with a conflict, while this is without one? or what should i do here?

@m4rch3n1ng
Copy link
Contributor Author

for now i did the second suggestion i had and implemented parsing of unsafe extern blocks without a conflict, as well as parsing safe fns and safe statics. lmk if this is fine or if i should do something different.

@m4rch3n1ng m4rch3n1ng changed the title parse unsafe extern blocks parse unsafe extern blocks without conflict, parse safe fn / static items Oct 6, 2025
@m4rch3n1ng m4rch3n1ng changed the title parse unsafe extern blocks without conflict, parse safe fn / static items parse unsafe extern blocks without conflict and safe fn / static items Oct 6, 2025
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.

bug: Missing parsing of safe/unsafe externs
3 participants