iains added inline comments.

================
Comment at: clang/include/clang/Basic/Module.h:523-524
+  std::string getPrimaryModuleInterfaceName() const {
+    std::string::size_type pos = Name.find(':');
+    if (pos == std::string::npos)
+      return Name;
----------------
iains wrote:
> urnathan wrote:
> > haven't you added an IsPartition flag to Module? can't you test that before 
> > searching? Also, Almost Always Auto?
> 1) The case we are dealing with is
> 
> we're building `A{,:B}`
> we encounter `:Part`
> 
> So we do not have a module for the importer ('cos we didn't build it yet) but 
> we need to know the primary module name for the imported partition (even to 
> be able to find it).
> 
> So it's a wee bit icky, but we are reduced to manipulating text (none of the 
> content of Module.h is available - we are looking at parser tokens).
> 
> 2) auto and StringRefs - yes probably I could do better.
> 
> The consolation is that this is not an action we'd reasonably expect to be 
> carried out many times c.f. other parser jobs - of course, I'll be proven 
> wrong and someone will import 10^6 partitions ....
> 
 
> (none of the content of Module.h is available - we are looking at parser 
> tokens).

maybe I retract that, at least partially,  we do not have a module (built) - 
but some of the base information is saved in sema when the module decl is 
built, so we probably do know if we are building `A:B` or just `A`. 

If we think that this could be a pain point - sema can cache the primary module 
name and we can then return a StringRef to that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118598

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

Reply via email to