ilya-biryukov added a comment.

In D67172#1663096 <https://reviews.llvm.org/D67172#1663096>, @sammccall wrote:

> LG, though if we can drop the struct and make MaxSuffixComponents a constant 
> it'd be simpler still.


Done.

> Sure. This is going to win a couple of percent I guess: for these cases we 
> care about walltime including rebuild and lit startup overhead. It's worth 
> having (and thanks for doing this!) but I don't think we should pay much 
> complexity.

Agreed.

> "we could try" sounds like we *don't* know how to eliminate it. Parsing 
> manpages aside, I thought the main problem was these symbols are nonstandard 
> and an infinitely portable qname->header mapping simply didn't exist.

I would expect qualified names to be more portable than paths, but might be 
mistaken. I recall that we might run into problems if folks define some names 
as macros instead of functions, but I would be interested to see how common is 
this.
We should probably write down the examples that are broken somewhere... It's 
hard to see why it doesn't work without having the concrete cases.
@hokein, do you have any examples that cannot be covered by qual-name-to-path 
mapping?

> Thanks, this looks a lot better!
>  I think it can be simplified a little further, as the suffix maps are 
> totally hardcoded now.

I think the simplest option is to always make the path suffix mapping available 
in the class.
That means we will always map the system paths to their shorter versions. It 
looks ok to me, but might make unit-testing a little more awkward.
WDYT?
I'll land this and send this out as a separate patch if everyone is happy with 
the approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67172



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

Reply via email to