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 diagnostic items for a few of core's builtin macros #117596

Merged
merged 1 commit into from
Nov 5, 2023

Conversation

thomcc
Copy link
Member

@thomcc thomcc commented Nov 5, 2023

Specifically, env, option_env, and include. There are a number of reasons why people might want to look at these in lints (For example, to ensure that things behave consistently, detect things that might make builds less reproducible, etc).

Concretely, in PL/Rust (well, plrustc) we have lints that forbid these (which I'd like to add to clippy as restriction lints eventually), and dylint also has lints that look for env!/option_env! (although perhaps not include), which would benefit from this.

My experience is that it's pretty annoying to (robustly) check uses of builtin macros without these IME, although that's perhaps just my own fault (e.g. I could be doing it wrong).

At @Nilstrieb's suggestion, I've added a comment that explains why these are here, even though they are not used in the compiler. This is mostly to discourage removal, although it's not a big deal if it happens (I'm certainly not suggesting the presence of these be in any way stable).


In theory this is a library PR (in that it's in library/core), but I'm going to roll compiler because the existence of this or not is much more likely something they care about rather than libs. Hopefully nobody objects to this.

r? compiler

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 5, 2023
@Noratrieb
Copy link
Member

r? Nilstrieb
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 5, 2023

📌 Commit 65bec86 has been approved by Nilstrieb

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2023
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#116017 (Don't pass `-stdlib=libc++` when building C files on macOS)
 - rust-lang#117524 (bootstrap/setup: create hooks directory if non-existing)
 - rust-lang#117588 (Remove unused LoadResult::DecodeIncrCache variant)
 - rust-lang#117596 (Add diagnostic items for a few of core's builtin macros)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a660516 into rust-lang:master Nov 5, 2023
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2023
Rollup merge of rust-lang#117596 - thomcc:core_macro_diag_items, r=Nilstrieb

Add diagnostic items for a few of core's builtin macros

Specifically, `env`, `option_env`, and `include`. There are a number of reasons why people might want to look at these in lints (For example, to ensure that things behave consistently, detect things that might make builds less reproducible, etc).

Concretely, in PL/Rust (well, `plrustc`) we have lints that forbid these (which I'd like to [add to clippy as restriction lints](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Landing.20a.20flotilla.20of.20lints.3F) eventually), and `dylint` also has [lints that look for `env!`/`option_env!`](https://github.com/trailofbits/dylint/blob/109a07e9f27a9651ef33b6677ccaddd21466e97a/examples/general/env_cargo_path/src/lib.rs) (although perhaps not `include`), which would benefit from this.

My experience is that it's pretty annoying to (robustly) check uses of builtin macros without these IME, although that's perhaps just my own fault (e.g. I could be doing it wrong).

At `@Nilstrieb's` suggestion, I've added a comment that explains why these are here, even though they are not used in the compiler. This is mostly to discourage removal, although it's not a big deal if it happens (I'm certainly not suggesting the presence of these be in any way stable).

---

In theory this is a library PR (in that it's in library/core), but I'm going to roll compiler because the existence of this or not is much more likely something they care about rather than libs. Hopefully nobody objects to this.

r? compiler
@rustbot rustbot added this to the 1.75.0 milestone Nov 5, 2023
@thomcc thomcc deleted the core_macro_diag_items branch November 5, 2023 21:09
bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Nov 6, 2023
84: Automated pull from upstream `master` r=Dajamante a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#117585
* rust-lang/rust#117576
* rust-lang/rust#96979
* rust-lang/rust#117191
* rust-lang/rust#117179
* rust-lang/rust#117574
* rust-lang/rust#117537
* rust-lang/rust#117608
  * rust-lang/rust#117596
  * rust-lang/rust#117588
  * rust-lang/rust#117524
  * rust-lang/rust#116017
* rust-lang/rust#117504
* rust-lang/rust#117469
* rust-lang/rust#116218
* rust-lang/rust#117589
* rust-lang/rust#117581
* rust-lang/rust#117503
* rust-lang/rust#117590
  * rust-lang/rust#117583
  * rust-lang/rust#117570
  * rust-lang/rust#117562
  * rust-lang/rust#117534
  * rust-lang/rust#116894
  * rust-lang/rust#110340
* rust-lang/rust#113343
* rust-lang/rust#117579
* rust-lang/rust#117094
* rust-lang/rust#117566
* rust-lang/rust#117564
  * rust-lang/rust#117554
  * rust-lang/rust#117550
  * rust-lang/rust#117343
* rust-lang/rust#115274
* rust-lang/rust#117540
* rust-lang/rust#116412
* rust-lang/rust#115333
* rust-lang/rust#117507
* rust-lang/rust#117538
  * rust-lang/rust#117533
  * rust-lang/rust#117523
  * rust-lang/rust#117520
  * rust-lang/rust#117505
  * rust-lang/rust#117434
* rust-lang/rust#117535
* rust-lang/rust#117510
* rust-lang/rust#116439
* rust-lang/rust#117508



Co-authored-by: Ben Wiederhake <[email protected]>
Co-authored-by: SabrinaJewson <[email protected]>
Co-authored-by: J-ZhengLi <[email protected]>
Co-authored-by: koka <[email protected]>
Co-authored-by: bjorn3 <[email protected]>
Co-authored-by: Joshua Liebow-Feeser <[email protected]>
Co-authored-by: lengyijun <[email protected]>
Co-authored-by: Zalathar <[email protected]>
Co-authored-by: Oli Scherer <[email protected]>
Co-authored-by: Philipp Krones <[email protected]>
Co-authored-by: y21 <[email protected]>
Co-authored-by: bors <[email protected]>
Co-authored-by: bohan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants