Implement basic bool and int formatting for diagnostics #4411
Conversation
| // If needed, the _full_ style string can be wrapped with `'` in order to | ||
| // preserve prefix or suffix whitespace (which is stripped by formatv). For | ||
| // example, `{0:' true | false '}` retains whitespace which would be dropped | ||
| // before `true` and after `false`. |
There was a problem hiding this comment.
Seems like a reasonable workaround. But the need to do this makes me sad.
There was a problem hiding this comment.
🤷
As discussed, we can revisit and see what std::format does whenever we're able to switch.
There was a problem hiding this comment.
Looks like std::format passes through the string with leading/trailing whitespace to the formatter.
So I'm happy to make the LLVM change to avoid trimming this so it works as desired. I'll follow-up here with the upstream PR/commit soon.
There was a problem hiding this comment.
We can update the implementation once LLVM has support; I'd rather not block this PR on that, to minimize merge conflicts.
There was a problem hiding this comment.
Posted upstream change here: llvm/llvm-project#112625
This comment was marked as resolved.
This comment was marked as resolved.
| // If needed, the _full_ style string can be wrapped with `'` in order to | ||
| // preserve prefix or suffix whitespace (which is stripped by formatv). For | ||
| // example, `{0:' true | false '}` retains whitespace which would be dropped | ||
| // before `true` and after `false`. |
There was a problem hiding this comment.
🤷
As discussed, we can revisit and see what std::format does whenever we're able to switch.
jonmeow
force-pushed
the
foramtting
branch
2 times, most recently
from
0d2bf66 to
1131c45
Compare
This is consistent with std::formatv and allows formatters to support a wider variety of use cases (like having a bare string in their formatter if that's useful, etc). Came up in the context of some Carbon diagnostic work here: carbon-language/carbon-lang#4411 (comment)
Building on #4411, replace format_provider uses (other than `TokenKind`, which is more on the okay side of things) Also does some edits to `ClassMemberDefinition` to try to better match diagnostic style
Building on #4411, avoid using StringLiteral in format strings. This includes a diagnostic check to prevent regressions (which is also how I gathered issues). Note, I haven't looked at `std::string` uses yet, but we might need things like that to be able to pass strings in code back to the user. StringLiteral though means that it's literally written down in the toolchain, at which point it should probably be written in the format string instead of separately. --------- Co-authored-by: Geoff Romer <[email protected]>
Note, this supports plurals, but doesn't apply it anywhere. I'm mainly doing that to demonstrate the approach regarding syntax. See format_providers.h for details.