while cfg!(unix) { ... } suggests to use loop instead

[oli-obk]

If any linting or suggestion is done in this case, it should suggest if cfg!(unix) { loop { ... } }

cc @jseyfried this has been in rustc forever, but clippy is only lately going crazy on it. Could this have come from your PR, too?

[jseyfried]

@oli-obk Hmm, cfg! shouldn't have been affected, but I'll investigate.

[oli-obk]

@jseyfried this is messing up clippy pretty bad. Any idea where to start looking?

[oli-obk]

So I tried to figure out what's going on, but I got lost in the code. As far as I can see it https://github.com/rust-lang/rust/blob/master/src/libsyntax/ext/expand.rs#L486 should be setting the expansion info. But it appears the span passed to https://github.com/rust-lang/rust/blob/master/src/libsyntax/ext/expand.rs#L492 doesn't contain the expansion info. So maybe we need to generate a new span here with the mark from https://github.com/rust-lang/rust/blob/master/src/libsyntax/ext/expand.rs#L465 ?

[jseyfried]

@oli-obk The span passed to https://github.com/rust-lang/rust/blob/master/src/libsyntax/ext/expand.rs#L492 is the call site span, so it shouldn't contain the expansion info.

The issue here is that cfg! is using the call site span directly in the expansion here (sp is call site span).

To fix this, the cfg macro should add expansion info to the call site span like format! does here. Adding that line in cfg.rs should fix this issue.

Are there other built-in macros that also have this problem in practice or is it just cfg!?

[oli-obk]

I'd assume others have it too, but I'll check.

So the passed call site span should only be used directly for error reporting, but never for creating ast items?

[jseyfried]

Right, unless you want an AST item with the raw (unexpanded) call site span -- I don't think we ever want that.