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

Add an ItemModifier syntax extension type #12617

Merged
merged 1 commit into from
Mar 11, 2014

Conversation

sfackler
Copy link
Member

Where ItemDecorator creates new items given a single item, ItemModifier
alters the tagged item in place. The expansion rules for this are a bit
weird, but I think are the most reasonable option available.

When an item is expanded, all ItemModifier attributes are stripped from
it and the item is folded through all ItemModifiers. At that point, the
process repeats until there are no ItemModifiers in the new item.

cc @huonw

@@ -37,6 +37,9 @@ pub struct MacroDef {
pub type ItemDecorator =
fn(&mut ExtCtxt, Span, @ast::MetaItem, @ast::Item, |@ast::Item|);

pub type ItemModifier =
Copy link
Member

Choose a reason for hiding this comment

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

Since you're adding a new type, could you add docs for it now?

@brson
Copy link
Contributor

brson commented Mar 1, 2014

What is the background for why we are doing this?

@sfackler
Copy link
Member Author

sfackler commented Mar 1, 2014

ItemDecorators lost the ability modify the tagged item in #12234.

There's an issue that I can't find right now to add an attribute to automatically insert extern function prefixes like this:

#[prefix("foobar_")]
extern {
    fn func_1();
    fn func_2();
}
// expands to...
extern {
    fn foobar_func_1();
    fn foobar_func_2();
}

@kballard and I were talking at the last meetup about another use case he had for wrapping functions in some bookkeeping code necessary for safety when interacting with LUA. I think the idea is something like this:

#[lua_safety]
fn foo() {
    //...
}
// expands to...
fn foo() {
    fn inner_foo() {
        //...
    }
    // LUA setup
    inner_foo();
    // LUA cleanup
}

He would know more details though.

@lilyball
Copy link
Contributor

lilyball commented Mar 1, 2014

Yeah, I was talking with @sfackler about this at the meetup. Right now I have a lua_extern!() macro that takes something that looks like a function declaration (with severe limitations, due to macro_rules!()). I asked about ItemDecorator, because I thought it would be much nicer if I could say #[lua_extern] fn foo() { ... } and have it do the appropriate transformation. But this would require the ability to modify (or more accurately, replace) the original item.

I also thought that this ability would allow #[cfg()] to be (re-)implemented using this same standard mechanism, instead of whatever mechanism it uses now, since all that really needs is the ability to remove the decorated item.

@lilyball
Copy link
Contributor

lilyball commented Mar 1, 2014

Actually, given my musings about #[cfg()], shouldn't ItemModifier return Option<@ast::Item>? It may also be useful to be able to return multiple items (unless there's some reason why that's a problem?), which would suggest instead ~[@ast::Item], although I don't know if forcing an allocation is something that is problematic.

@sfackler
Copy link
Member Author

sfackler commented Mar 1, 2014

I'd hesitate to return a ~[@ast::Item] because the ItemModifier can always return an item with an ItemDecorator attached to create the extra items. It also make it a bit confusing as to what happens if we have #[a] #[b] struct Foo {} and the a extension produces multiple results. Do we pass all of them through b? The same kind of goes for returning an option. I'm also not sure if there's a use case for returning no items other than cfg. Again, the ItemModifier can always attach a cfg annotation to get the thing to go away.

@lilyball
Copy link
Contributor

lilyball commented Mar 1, 2014

Returning an item with an ItemDecorator attached didn't occur to me. Possibly a bit confusing though.

As for returning none, if my ItemModifier determines it doesn't want any item, returning something with #[cfg(blah)] is downright odd.

@alexcrichton
Copy link
Member

ping @sfackler, with @huonw's comment addressed, I think this may be mergeable.

The API of libsyntax will likely be #[experimental] for eternity. We may not have hit the sweet spot in terms of syntax extensions, but it seems like this may be a legitimate use case. I think that we still have design space to explore with exactly how the syntax extension api looks.

@brson, are you ok with merging under that logic?

@brson
Copy link
Contributor

brson commented Mar 10, 2014

Yes. +1
On Mar 10, 2014 12:08 AM, "Alex Crichton" notifications@github.com wrote:

ping @sfackler https://github.com/sfackler, with @huonwhttps://github.com/huonw's
comment addressed, I think this may be mergeable.

The API of libsyntax will likely be #[experimental] for eternity. We may
not have hit the sweet spot in terms of syntax extensions, but it seems
like this may be a legitimate use case. I think that we still have design
space to explore with exactly how the syntax extension api looks.

@brson https://github.com/brson, are you ok with merging under that
logic?

Reply to this email directly or view it on GitHubhttps://github.com//pull/12617#issuecomment-37157509
.

@sfackler
Copy link
Member Author

Sounds good. I'll update it tomorrow night.

@sfackler
Copy link
Member Author

Updated

Where ItemDecorator creates new items given a single item, ItemModifier
alters the tagged item in place. The expansion rules for this are a bit
weird, but I think are the most reasonable option available.

When an item is expanded, all ItemModifier attributes are stripped from
it and the item is folded through all ItemModifiers. At that point, the
process repeats until there are no ItemModifiers in the new item.
bors added a commit that referenced this pull request Mar 11, 2014
Where ItemDecorator creates new items given a single item, ItemModifier
alters the tagged item in place. The expansion rules for this are a bit
weird, but I think are the most reasonable option available.

When an item is expanded, all ItemModifier attributes are stripped from
it and the item is folded through all ItemModifiers. At that point, the
process repeats until there are no ItemModifiers in the new item.

cc @huonw
@bors bors closed this Mar 11, 2014
@bors bors merged commit eb4cbd5 into rust-lang:master Mar 11, 2014
@sfackler sfackler deleted the item-modifier branch May 15, 2014 05:03
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 4, 2024
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local

Fixes rust-lang#12616

`Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE

changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 25, 2024
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local

Fixes rust-lang#12616

`Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE

changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Getting action download info
Download action repository 'msys2/setup-msys2@v2.22.0' (SHA:cc11e9188b693c2b100158c3322424c4cc1dadea)
Download action repository 'actions/checkout@v4' (SHA:1d96c772d19495a3b5c517cd2bc0cb401ea0529f)
Download action repository 'actions/upload-artifact@v3' (SHA:a8a3f3ad30e3422c9c7b888a15615d19a852ae32)
Complete job name: PR - mingw-check-tidy
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
  TOOLSTATE_REPO: https://github.com/rust-lang-nursery/rust-toolstate
  CACHE_DOMAIN: ci-caches.rust-lang.org
  IMAGE: mingw-check-tidy
##[endgroup]
fatal: unknown commit 53fd98ca776cb875bc9e5514f56b52eb74f9e7a9
All commits in `HEAD` are present in `master`
src/ci/scripts/verify-stable-version-number.sh
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
---
COPY scripts/sccache.sh /scripts/
RUN sh /scripts/sccache.sh

COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt \
    && pip3 install virtualenv
COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---

#12 [5/8] COPY host-x86_64/mingw-check/reuse-requirements.txt /tmp/
#12 DONE 0.0s

#13 [6/8] RUN pip3 install --no-deps --no-cache-dir --require-hashes -r /tmp/reuse-requirements.txt     && pip3 install virtualenv
#13 0.435   Downloading binaryornot-0.4.4-py2.py3-none-any.whl (9.0 kB)
#13 0.520 Collecting boolean-py==4.0
#13 0.526   Downloading boolean.py-4.0-py3-none-any.whl (25 kB)
#13 0.543 Collecting chardet==5.1.0
---
#13 3.740 Building wheels for collected packages: reuse
#13 3.741   Building wheel for reuse (pyproject.toml): started
#13 4.067   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 4.068   Created wheel for reuse: filename=reuse-1.1.0-cp310-cp310-manylinux_2_35_x86_64.whl size=181117 sha256=f5f58750481f69515c2c0d1d503daf565e2565c370d07fc6aeb95fe3498b4269
#13 4.068   Stored in directory: /tmp/pip-ephem-wheel-cache-jk8zf515/wheels/c2/3c/b9/1120c2ab4bd82694f7e6f0537dc5b9a085c13e2c69a8d0c76d
#13 4.071 Installing collected packages: boolean-py, binaryornot, setuptools, reuse, python-debian, markupsafe, license-expression, jinja2, chardet
#13 4.093   Attempting uninstall: setuptools
#13 4.093     Found existing installation: setuptools 59.6.0
#13 4.095     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#13 4.095     Not uninstalling setuptools at /usr/lib/python3/dist-packages, outside environment /usr
#13 4.095     Can't uninstall 'setuptools'. No files were found to uninstall.
#13 4.767 Successfully installed binaryornot-0.4.4 boolean-py-4.0 chardet-5.1.0 jinja2-3.1.2 license-expression-30.0.0 markupsafe-2.1.1 python-debian-0.1.49 reuse-1.1.0 setuptools-66.0.0
#13 4.768 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#13 5.281 Collecting virtualenv
#13 5.332   Downloading virtualenv-20.26.0-py3-none-any.whl (3.9 MB)
#13 5.462      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.9/3.9 MB 30.7 MB/s eta 0:00:00
#13 5.514 Collecting platformdirs<5,>=3.9.1
#13 5.521   Downloading platformdirs-4.2.1-py3-none-any.whl (17 kB)
#13 5.554 Collecting filelock<4,>=3.12.2
#13 5.561   Downloading filelock-3.13.4-py3-none-any.whl (11 kB)
#13 5.581 Collecting distlib<1,>=0.3.7
#13 5.590   Downloading distlib-0.3.8-py2.py3-none-any.whl (468 kB)
#13 5.600      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 468.9/468.9 KB 54.8 MB/s eta 0:00:00
#13 5.687 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#13 5.853 Successfully installed distlib-0.3.8 filelock-3.13.4 platformdirs-4.2.1 virtualenv-20.26.0
#13 DONE 5.9s

#14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#14 DONE 0.0s
---
DirectMap4k:      194496 kB
DirectMap2M:     6096896 kB
DirectMap1G:    12582912 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint
    Finished dev [unoptimized] target(s) in 0.03s
##[endgroup]
downloading https://ci-artifacts.rust-lang.org/rustc-builds-alt/7aa1de7ed84e77aa22d1428128fcd4089a3abd13/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz
extracting /checkout/obj/build/cache/llvm-7aa1de7ed84e77aa22d1428128fcd4089a3abd13-true/rust-dev-nightly-x86_64-unknown-linux-gnu.tar.xz to /checkout/obj/build/x86_64-unknown-linux-gnu/ci-llvm
---
   Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
    Finished release [optimized] target(s) in 27.13s
##[endgroup]
tidy check
tidy error: following path contains more than 866 entries, you should move the test to some relevant subdirectory (current: 867): /checkout/tests/ui
removing old virtual environment
removing old virtual environment
creating virtual environment at '/checkout/obj/build/venv' using 'python3.10'
Requirement already satisfied: pip in ./build/venv/lib/python3.10/site-packages (24.0)
Collecting black==23.3.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 7))
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 1.7/1.7 MB 17.6 MB/s eta 0:00:00
Collecting click==8.1.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 34))
  Downloading click-8.1.3-py3-none-any.whl (96 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 96.6/96.6 kB 25.3 MB/s eta 0:00:00
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 96.6/96.6 kB 25.3 MB/s eta 0:00:00
Collecting importlib-metadata==6.7.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 38))
  Downloading importlib_metadata-6.7.0-py3-none-any.whl (22 kB)
Collecting mypy-extensions==1.0.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 42))
  Downloading mypy_extensions-1.0.0-py3-none-any.whl (4.7 kB)
Collecting packaging==23.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 46))
  Downloading packaging-23.1-py3-none-any.whl (48 kB)
Collecting pathspec==0.11.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 50))
  Downloading pathspec-0.11.1-py3-none-any.whl (29 kB)
Collecting platformdirs==3.6.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 54))
  Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB)
  Downloading platformdirs-3.6.0-py3-none-any.whl (16 kB)
Collecting ruff==0.0.272 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 58))
  Downloading ruff-0.0.272-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (5.9 MB)
Collecting tomli==2.0.1 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 77))
  Downloading tomli-2.0.1-py3-none-any.whl (12 kB)
Collecting typed-ast==1.5.4 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 81))
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
  Downloading typed_ast-1.5.4-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl (877 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 877.7/877.7 kB 85.2 MB/s eta 0:00:00
Collecting typing-extensions==4.6.3 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 107))
  Downloading typing_extensions-4.6.3-py3-none-any.whl (31 kB)
Collecting zipp==3.15.0 (from -r /checkout/src/tools/tidy/config/requirements.txt (line 114))
  Downloading zipp-3.15.0-py3-none-any.whl (6.8 kB)
Installing collected packages: zipp, typing-extensions, typed-ast, tomli, ruff, platformdirs, pathspec, packaging, mypy-extensions, click, importlib-metadata, black
Successfully installed black-23.3.0 click-8.1.3 importlib-metadata-6.7.0 mypy-extensions-1.0.0 packaging-23.1 pathspec-0.11.1 platformdirs-3.6.0 ruff-0.0.272 tomli-2.0.1 typed-ast-1.5.4 typing-extensions-4.6.3 zipp-3.15.0
some tidy checks failed
Build completed unsuccessfully in 0:00:59
  local time: Thu Apr 25 12:04:48 UTC 2024
  network time: Thu, 25 Apr 2024 12:04:48 GMT

flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 25, 2024
avoid an ICE in `ptr_as_ptr` when getting the def_id of a local

Fixes rust-lang#12616

`Res::def_id` can panic, so avoid calling it in favor of `opt_def_id`, so we can gracefully handle resolutions that don't have a `DefId` (e.g. local variables) and get a false negative in the worst case, rather than an ICE

changelog: Fix ICE in [`ptr_as_ptr`] when the cast expression is a function call to a local variable
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.

None yet

7 participants