[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 4 11:54:07 PDT 2022


sammccall added a comment.

Sorry, Monday was a holiday here...

I don't think unconditionally including the filename in printQualifiedName() is great for tools, it can be unreasonably long and is generally just noise when shown in the context of that file. I'm surprised that USR generation + that clangd test are the only things that broke! Poor test coverage in the tools, I think :-(
If the intent is to change this for diagnostics only, can it be behind a PrintingPolicy flag?

The reason USR generation broke is it was deliberately testing/handling, see `bool USRGenerator::EmitDeclName()` in USRGeneration.cpp. (I'm not very familiar with the implementation, a bit more so with how its results are used). This isn't a good reason to keep the output as `""` forever - we should probably change it to detect empty names explicitly instead.



================
Comment at: clang/test/Index/usrs.m:113
 // CHECK: usrs.m c:usrs.m at 102@F at my_helper@y Extent=[3:36 - 3:41]
-// CHECK: usrs.m c:@Ea at ABA Extent=[5:1 - 8:2]
-// CHECK: usrs.m c:@Ea at ABA@ABA Extent=[6:3 - 6:6]
-// CHECK: usrs.m c:@Ea at ABA@CADABA Extent=[7:3 - 7:9]
-// CHECK: usrs.m c:@Ea at FOO Extent=[10:1 - 13:2]
-// CHECK: usrs.m c:@Ea at FOO@FOO Extent=[11:3 - 11:6]
-// CHECK: usrs.m c:@Ea at FOO@BAR Extent=[12:3 - 12:6]
-// CHECK: usrs.m c:@SA at MyStruct Extent=[15:9 - 18:2]
-// CHECK: usrs.m c:@SA at MyStruct@FI at wa Extent=[16:3 - 16:9]
-// CHECK: usrs.m c:@SA at MyStruct@FI at moo Extent=[17:3 - 17:10]
+// CHECK: usrs.m c:@E at enum (unnamed at {{.*}}) Extent=[5:1 - 8:2]
+// CHECK: usrs.m c:@E at enum (unnamed at {{.*}})@ABA Extent=[6:3 - 6:6]
----------------
This changes the semantics of the USRs: previously if an anonymous enum moved (e.g. because a file was edited) it retains its identity as long as the first enumerator keeps its name.

The equivalence classes of decls under USR mapping is important for some tools: I don't know all the ways Apple/XCode stuff uses it, but refactoring tools, clangd indexing etc will be affected by this.

(Occasional changes to the concrete USR values are manageable, basically we need to invalidate indexes that are otherwise usable across versions)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134813/new/

https://reviews.llvm.org/D134813



More information about the cfe-commits mailing list