[PATCH] D32073: [ProfileData] Support cross target binary reading for coverage tool

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In https://reviews.llvm.org/D32073#727016, @davidxl wrote:

> The main client (the compiler) should not need to know the details so a higher level interface taking 'Module' should be better.


I see your point, but taking a 'Module' doesn't seem to be nicer, because it forces us to expose more APIs which are very slightly different from each other. This leaks details because in order to pick the right API, you need to know the low level details (e.g whether or not segment prefixes are needed).

>> Wdyt of removing all of the get*SectionName helpers, and just exposing: getInstrProfSectionName(InstrProfSectKind IPSK, ObjectFileKind OFK, bool AddSegmentPrefix). It will make client code more verbose but very clear / unambiguous.
> 
> It is the host tool that need to use the lower level interface. For  the later use, there was never a need for segment prefix, so adding the flexibility is not necessary.

We could set 'bool AddSegmentPrefix = true' by default. This still provides some extra flexibility, but the right option is picked by default, so there should be less confusion.

If you'd prefer to extend the set of get*SectionName() functions, I just ask that the names of the "getXSectionName(bool)" and "getXSectionName(Module *)" functions be made different.


https://reviews.llvm.org/D32073