dblaikie added a comment. In D110044#3009201 <https://reviews.llvm.org/D110044#3009201>, @aaron.ballman wrote:
>> 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? 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. ================ Comment at: clang/lib/AST/Type.cpp:3045 case NullPtr: - return "nullptr_t"; + return "std::nullptr_t"; case Overload: ---------------- aaron.ballman wrote: > Should this be `::std::nullptr_t` to differentiate it from odd things like: > ``` > namespace my { > namespace std { > class nullptr_t {}; > } > } > ``` 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110044/new/ https://reviews.llvm.org/D110044 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits