bnbarham added inline comments.

================
Comment at: clang/lib/Index/USRGeneration.cpp:1032
+  case TemplateArgument::UncommonValue:
+    // FIXME: Visit value.
+    break;
----------------
bolshakov-a wrote:
> bnbarham wrote:
> > akyrtzi wrote:
> > > erichkeane wrote:
> > > > bolshakov-a wrote:
> > > > > aaron.ballman wrote:
> > > > > > Any particular reason this isn't being handled now?
> > > > > I need some guidance here. Which characters are allowed in the USR? 
> > > > > Could the mangling algorithm from 
> > > > > `CXXNameMangler::mangleValueInTemplateArg` be moved into some common 
> > > > > place and reused here?
> > > > I have no idea what is valid here.  BUT @akyrtzi and @gribozavr (or 
> > > > @gribozavr2 ?) seem to be the ones that touch these files the most?
> > > Adding @bnbarham to review the `Index` changes.
> > Just visiting the underlying type seems reasonable, ie. 
> > `VisitType(Arg.getUncommonValueType());`. If it needed to be differentiated 
> > between a `TemplateArgument::Type` you could add a prefix character (eg. 
> > `U`), but that doesn't seem needed to me.
> Doesn't the holded value be added so as to distinguish e.g. `Tpl<1.5>` from 
> `Tpl<2.5>`?
Ah I see, yeah, we would. And there's no USR generation for APValue currently, 
which I assume is why your original question came up.

In general a USR just wants to uniquely identify an entity across compilations 
and isn't as restricted as the mangled name. For basically everything but 
`LValue` it seems like you'd be fine to print the value (for eg. int, float, 
etc), visit the underlying type (array, vector), or the visit the underlying 
decl (struct, union, member pointer). That's almost true for `LValue` as well, 
just with the extra parts that are also added to the ODR hash.

Alternatively, you could also just print the hash from `Profile` with the same 
handling as ODR hash. Worst case we'd accidentally merge specializations, but 
if that's good enough for the ODR hash it's probably good enough here as well.


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

https://reviews.llvm.org/D140996

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to