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

Reply via email to