Since the addition of TranslateArg, I don't think this type is going to
go away (cutting a TODO). Refactoring names slightly to fit the current
role, and adding const to ConvertLocation.
This gets to a lifetime subtlety, particularly with things like the
sorting diagnostic consumer that delay output. In order to reduce the
chance of accidental references, disallow StringRef in the diagnostics.
For example:
```
./toolchain/diagnostics/diagnostic_emitter.h:162:5: error: static_assert failed due to requirement '!std::is_same_v<llvm::StringRef, llvm::StringRef>' "Use std::string or llvm::StringLiteral for diagnostic lifetimes."
static_assert(
^
toolchain/check/convert.cpp:477:11: note: in instantiation of member function 'Carbon::Internal::DiagnosticBase<std::string, std::string, llvm::StringRef>::DiagnosticBase' requested here
CARBON_DIAGNOSTIC(StructInitMissingFieldInConversion, Error,
^
./toolchain/diagnostics/diagnostic_emitter.h:47:7: note: expanded from macro 'CARBON_DIAGNOSTIC'
::Carbon::Internal::DiagnosticBase<__VA_ARGS__>( \
^
```
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Rationale: this convention avoids forcing closely-related code to be far
apart in the namespace hierarchy, and vice versa. By the same token, it
makes the namespace hierarchy more consistent with the directory
hierarchy.
A lot of this is more boring "remove unused header", plus some other
minor cleanups. I think the most significant changes were:
- yaml_test_helpers.cpp is doing a switch on an unsigned int, comparing
to enum values.
-
[bugprone-switch-missing-default-case](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/switch-missing-default-case.html)
is unhappy with EnumBase, but correctly identified
yaml_test_helpers.cpp, so I'm opting to address it rather than disabling
it even though it needs NOLINT in several locations as a result, in
addition to what I think are some low-value `default` cases. I'd be fine
going the other way with this too and disabling it globally (I could see
it being noisier in the explorer).
- MarkInitializerFor swaps the argument names between the .h and .cpp. I
think the .cpp had the order as intended.
- There's a new-ish
[performance-enum-size](https://clang.llvm.org/extra/clang-tidy/checks/performance/enum-size.html)
which I'm basically treating as "add int8_t to enums".
My main motivation here is to just clean up as many of these as I can so
that I stop seeing them in vscode.
This also makes the filename a reference to the buffer since the line
seems better to have as a reference (versus copying a string per error).
Most tests now have different line deltas due to the extra output, but
the actual errors should overall stay the same.
Some of the error locations look like they could be improved, but this
change is only making it clear where they were before.
Jon Ross-Perkins
Geoff Romer
