[PATCH] D46281: [clang-doc] Attaching a name to reference data

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.


================
Comment at: clang-doc/BitcodeWriter.cpp:382
+  emitRecord(R.USR, REFERENCE_USR);
+  emitRecord(R.Name, REFERENCE_NAME);
+  emitRecord((unsigned)R.RefType, REFERENCE_TYPE);
----------------
lebedev.ri wrote:
> juliehockett wrote:
> > lebedev.ri wrote:
> > > >>! In D46281#1083806, @lebedev.ri wrote:
> > > > Global question: you are using `NamedDecl::getNameAsString()`, and passing it as `StringRef`.
> > > > You have looked at this with ASAN, and it's ok?
> > > > 
> > > > Also, have you considered using the `NamedDecl::getName()`, which already returns `StringRef`.?
> > > 
> > > Hm, looking at those two functions, not sure `NamedDecl::getName()` will work here.
> > > Alternatively, have you considered just making this `Name` field store `DeclarationName`,
> > > and call `getNameAsString()` only here?
> > The field stores it as a string? So the name string is copied into the info data structure itself -- this is so that the backend need have no knowledge of the AST to do its job.
> Oh i see, forgot about that.
One more thing to consider, then: does it always receive `std::string`?
(from `getNameAsString()` and others, maybe).

If yes, maybe it would be simpler, more efficient (Since you could `std::move()` it,
no copying/allocating needed.) to just store the `std::string` itself.



https://reviews.llvm.org/D46281