We can now remove the specific handling of NestedNameQualifiers as
it's now handled by the type printer as of r349729.
Details
- Reviewers
aaron.ballman • Quuxplusone
Diff Detail
- Repository
- rC Clang
- Build Status
Buildable 26179 Build 26178: arc lint + arc unit
Event Timeline
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 | Any idea why the std:: was dropped here? |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 | NestedNameSpecifier::print() explicitly does: PrintingPolicy InnerPolicy(Policy); InnerPolicy.SuppressScope = true; |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 | Ah, good point, but is that a good behavioral change? I slightly prefer printing the namespace name there -- it will likely be redundant information most of the time, but when the namespace actually matters, having it printed could save someone a lot of head scratching. |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 |
I tend to agree, so it's more a trade-off of code complexity vs better diagnostic - I tend to err on the side of simplifying the code :) Another option is to add yet another boolean to PrintingPolicy, but I htink this is too narrow a use case. |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 | Heh, I tend to err on the side of helping the user unless the code will be truly awful. I agree that another option on PrintingPolicy may not be the best approach. Do you know why the namespace is being suppressed in the first place? Another option would be to always print the namespace, but I don't know what that might regress (if anything). |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 | Unfortunately always printing the qualification breaks 30 tests, including some in a very bad way: /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/AST/ast-print-out-of-line-func.cpp:29:11: error: CHECK: expected string not found in input // CHECK: void Wrapper::Inner::operator+=(int) ^ <stdin>:14:43: note: scanning from here ns::Wrapper::ns::Wrapper::Inner::Inner() { ^ <stdin>:16:19: note: possible intended match here void ns::Wrapper::ns::Wrapper::Inner::operator+=(int) { |
test/SemaCXX/static-assert.cpp | ||
---|---|---|
130 | That looks like a bug with the printer, no? As it stands, I think this is a regression with the diagnostic printing. I think it would be reasonable if the namespace were dropped only when the namespace reduces clarity (e.g., there are no other conflicting names in other namespaces, so the namespace is superfluous), but I don't think that's what's happening here. |
Any idea why the std:: was dropped here?