[llvm] Stop abusing Twine in DiagnosticInfo (PR #136371)
Justin Bogner via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 19 15:56:54 PDT 2025
bogner wrote:
> Can we just get a warning on the bad usage instead?
I'd be perfectly happy for this to just warn on misuse, but I don't think it's possible. Essentially we'd need to be able to warn on creating any of these `DiagnosticInfo*` classes on the stack, as the problematic pattern is as follows:
```c++
DiagnosticInfo Foo(...);
Ctx.diagnose(Foo);
```
The only way to correctly use these diagnostic classes is within the expression that calls `LLVMContext::diagnose`. This is the same as the problem with `Twine` itself, where we also need to ensure we're never doing this. IIRC we've discussed creating a warning for that too, but nobody came up with anything satisfactory. The only real difference is really that it's well known how `Twine` works and where it's dangerous, so people tend to catch misuses in code review.
If we want to make these interfaces "safe", we either need to move them away from `Twine`, or make them actually copy and store the string. If we *don't* want to make these safe because we care about the miniscule amount of extra stack space this approach uses in error paths, then I guess this isn't necessary and we can just continue to live with watching people debug the subtle build configuration and platform specific bugs that occur with these types.
https://github.com/llvm/llvm-project/pull/136371
More information about the llvm-commits
mailing list