[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