This improves diagnostic (& important to me, DWARF) accuracy - otherwise
there could be ambiguities between "std::nullptr_t" and some user-defined
type that's /actually/ "nullptr_t" defined in the global namespace.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This improves diagnostic (& important to me, DWARF) accuracy
FWIW, I don't think the diagnostic particularly needs more accuracy here -- I think users know what nullptr_t type is being referred to without the full qualification because of other contextual clues in the diagnostic text. However, I'm not opposed to the changes as I don't think they make anything worse. But I didn't see any tests for DWARF behavioral changes, are those changes in a future patch, or should there be some more test coverage?
| clang/lib/AST/Type.cpp | ||
|---|---|---|
| 3045 | Should this be ::std::nullptr_t to differentiate it from odd things like: namespace my {
namespace std {
class nullptr_t {};
}
} | |
Oh, happy to add a dedicated DWARF test, but hadn't originally planned on it - since it relies on all the same type printing infrastructure anyway. I'll see about adding one.
| clang/lib/AST/Type.cpp | ||
|---|---|---|
| 3045 | I was hoping not to get overly pedantic - I think clang omits the global namespace scope when naming other types in namespaces? Yeah: scope.cpp:5:5: error: invalid operands to binary expression ('int' and 'ns::inner')
1 + ns::inner();
~ ^ ~~~~~~~~~~~So this seems consistent with that, at least. That's true also when rendering template parameter type names, etc, so far as I know. | |
| clang/lib/AST/Type.cpp | ||
|---|---|---|
| 3045 | Thanks for the review! | |
Should this be ::std::nullptr_t to differentiate it from odd things like:
namespace my { namespace std { class nullptr_t {}; } }