ilya-biryukov added a comment.

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

> I do wonder whether we're microoptimizing for the tests too much, I don't 
> think 5% on the tests is worth very much in itself, unless it's speeding up 
> real workloads or improving the code (it may well be).


Even though tests don't parse real C++ programs, I wouldn't call it a 
micro-optimization. Spending 5% on such a low-level operation is not ok, small 
things like that can add up and have a subtle effect on performance.
If there's an issue with making the code more complex, I'm happy to explore 
other ways to make it simpler. What would convince you that's a cleanup that 
improves things? What parts of the current version do you think are problematic?

> I'm nervous about requiring langopts, it's convenient in tests/prototypes/etc 
> to be able to default-construct CanonicalIncludes (e.g. a test that doesn't 
> actually care about standard library mappings).

We can have a factory method that constructs with default lang opts for tests, 
I don't think this would be an issue in the actual application code. In fact, I 
think it'll make the application code better.


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