[PATCH] D84599: [clang-index] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

(Disclaimer: I am not at all familiar with this code)

**A few notes in no particular order**:
Some entities with special names should presumably be mangled, but are currently not. Example:

  namespace special_names {
  struct S { operator int(); };
  int operator""_udl(const char *);
  template <typename T> struct ST {};
  template <typename T> ST(T) -> ST<int>;
  }

results in:

  // CHECK: usrs.cpp c:@N at special_names Extent=[120:1 - 127:2]
  // CHECK: usrs.cpp c:@N at special_names@S at S Extent=[121:1 - 123:2]
  // CHECK: usrs.cpp c:@N at special_names@S at S@F at operator int# Extent=[122:3 - 122:17]
  // CHECK: usrs.cpp c:@N at special_names@F at operator""_udl#*1C# Extent=[124:1 - 124:33]
  // CHECK: usrs.cpp c:@N at special_names@ST>1#T at ST Extent=[125:1 - 125:35]
  // CHECK: usrs.cpp c:usrs.cpp at 2114 Extent=[125:11 - 125:21]
  // CHECK: usrs.cpp c:@N at special_names@FT@>1#T<deduction guide for ST>#t0.0#$@N at special_names@S at ST>#I# Extent=[126:1 - 126:38]
  // CHECK: usrs.cpp c:usrs.cpp at 2149 Extent=[126:10 - 126:20]

Some unnamed entities are already handled. In particular:

- Unnamed bit-fields are ignored (`VisitFieldDecl`). But note that the comment is wrong; the fields in a lambda class are unnamed. Should they be mangled?
- Anonymous namespaces are handled (`VisitNamespaceDecl`).
- The name of template parameters is not included in the mangling (`VisitTemplateParameterList`) so unnamed template parameters are already indirectly handled.
- For anonymous records, no USR is generated for the unnamed variable but an USR is generated for the anonymous record. However it can ambiguous in some cases:

  namespace anonymous_records {
  static union { int i0; };
  static union { int i1; };
  struct S { union { int j; }; };
  struct T { struct { int k; }; };
  }

results in:

  // CHECK: usrs.cpp c:@N at anonymous_records Extent=[136:1 - 141:2]
  // CHECK: usrs.cpp c:usrs.cpp at N@anonymous_records at Ua Extent=[137:8 - 137:25]
  // CHECK: usrs.cpp c:usrs.cpp at N@anonymous_records at Ua@FI at i0 Extent=[137:16 - 137:22]
  // CHECK: usrs.cpp c:usrs.cpp at N@anonymous_records at Ua Extent=[138:8 - 138:25]
  // CHECK: usrs.cpp c:usrs.cpp at N@anonymous_records at Ua@FI at i1 Extent=[138:16 - 138:22]
  // CHECK: usrs.cpp c:@N at anonymous_records@S at S Extent=[139:1 - 139:31]
  // CHECK: usrs.cpp c:@N at anonymous_records@S at S@Ua Extent=[139:12 - 139:28]
  // CHECK: usrs.cpp c:@N at anonymous_records@S at S@Ua at FI@j Extent=[139:20 - 139:25]
  // CHECK: usrs.cpp c:@N at anonymous_records@S at T Extent=[140:1 - 140:32]
  // CHECK: usrs.cpp c:@N at anonymous_records@S at T@Sa Extent=[140:12 - 140:29]
  // CHECK: usrs.cpp c:@N at anonymous_records@S at T@Sa at FI@k Extent=[140:21 - 140:26]

- For unnamed enumerations, the name of the first enumerator is used.
- No USRs are currently generated for  `MSGuidDecl` and `DecompositionDecl` anyway since they are not visited:

  namespace decomposition_decl {
  struct S { int i = 0; };
  void Test() { auto [x] = S(); } 
  }

results in:

  // CHECK: usrs.cpp c:@N at decomposition_decl Extent=[150:1 - 155:2]
  // CHECK: usrs.cpp c:@N at decomposition_decl@S at S Extent=[151:1 - 151:24]
  // CHECK: usrs.cpp c:@N at decomposition_decl@S at S@FI at i Extent=[151:12 - 151:21]
  // CHECK: usrs.cpp c:@N at decomposition_decl@F at Test# Extent=[152:1 - 154:2]
  // CHECK: usrs.cpp c:usrs.cpp at 2610@N at decomposition_decl@F at Test#@x Extent=[153:9 - 153:10]

**Conclusion**: I think that USR generation should not depend in general on the pretty-printed name of entities. Therefore something like the following should be done:

- Instead of using `NamedDecl::printName` in `EmitDeclName`, use `NamedDecl::getName` for simple names and handle separately each kind of special names.
- For unnamed entities for which an USR should be generated, implement the corresponding `Visit*`. The `Visit*` function should not depend on the pretty-printed name.

Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84599