Fix some unexpected_cfg lint warnings #5052
Conversation
Manishearth
force-pushed
the
unexpected_cfgs
branch
from
36ee2e1 to
1f2abd0
Compare
Manishearth
force-pushed
the
unexpected_cfgs
branch
from
1f2abd0 to
1d5a03a
Compare
Manishearth
changed the title
Manishearth
requested review from
aethanyc,
dminor,
echeran,
eggrobin,
hsivonen,
makotokato,
nordzilla,
pdogr,
sffc and
zbraniecki
as code owners
Gonna make this change in this PR to try and get CI passing again. We can figure out a longer term macro solution later: this is extremely hacky and works in an unintuitive way. |
Manishearth
requested review from
robertbastian
and removed request for
aethanyc,
dminor,
echeran,
eggrobin,
hsivonen,
makotokato,
nordzilla,
pdogr and
zbraniecki
| options: skip, | ||
| error: DataError, | ||
| #[cfg(skip)] | ||
| #[cfg(not(all()))] |
There was a problem hiding this comment.
Suggestion: these cfg(skip)s in the invocations of gen_any_buffer_data_constructors! could be better modeled by changing the macro itself to not emit these functions.
There was a problem hiding this comment.
Yes, that's what I said in the comments, but I didn't want to figure that out whilst fixing nightly CI.
It appears we are required to include the lint level. Hm. |
|
Can the level be "notice" or "ignore" or something? |
|
We definitely want the lint for unknown directives. The suppression is only for the few we include intentionally. (I don't know why the issue closed and reopened) |
|
@sffc I don't understand, I had previously set the lint to "warn" and you didn't want that, what is the difference between "warn" and what you mean by "notice"? |
This reverts commit 0fce3c8.
|
This fixes all but one case, and that needs to be fixed by improving the macro first. I'm working on that as a separate PR for now. |
| segmenter_lstm = { tagged = "_segmenter_lstm_tag_" } | ||
|
|
||
| [lints.rust] | ||
| unexpected_cfgs = { level = "warn", check-cfg = ['cfg(icu4x_custom_data)'] } |
There was a problem hiding this comment.
This is emitted by the build script, why do we need to suppress it?
There was a problem hiding this comment.
It needs to be unconditionally emitted with a "this cfg exists" directive
| unexpected_cfgs = { level = "warn", check-cfg = ['cfg(icu4x_custom_data)'] } | ||
|
|
||
| [dependencies] | ||
|
|
|
I resolved the comments but I also undid the cfg(skip) changes when fixing merge conflicts since that's going to conflict more. |
3 participants
Latest Rust has a bunch of warnings around unexpected
cfgkeys, to avoid mistakes and encourage use of features. This can be worked around by documenting deliberate CFG keys in Cargo.toml (or by emitting them from the build script, which I'd rather not introduce).This doesn't fix all of them, we have a ton of
cfg(skip)that we should fix. Perhapscfg(not(any()))for now, though really we should tweak the ctor macros to not need this, the cfg is a hack.