[PATCH] D120334: [NFC][Lexer] Use more appropriate LangOptionsBase type for Lexer::LangOpts

Jan Svoboda via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 22 12:14:19 PST 2022


jansvoboda11 added a comment.

In D120334#3338314 <https://reviews.llvm.org/D120334#3338314>, @yurai007 wrote:

> In D120334#3337961 <https://reviews.llvm.org/D120334#3337961>, @jansvoboda11 wrote:
>
>> The performance implications are pretty interesting! Have you tried avoiding the copy altogether by just storing the reference?
>
> Yes, actually that was my first attempt but I failed miserably. LangOpts::LineComment is mutated in Lexer so LangOpts cannot be simply const&. Making it non-const reference is technically doable with some extra adjustment for Lexer constructors.
> However I'm worried a bit how it would impact external LangOpts users after mutating LineComment member by Lexer.  As long as LangOpts acts as simple cache it's easy to reason about but when we start to share it with Lexer callers I'm not longer sure. That's why eventually base class approach was chosen.

Got it. It seems like getting rid of the `LineComment` mutation would be pretty straightforward though. I don't expect any big perf win by replacing copy of a handful of bitfields with indirection, but it would be interesting to see the experiment nonetheless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120334



More information about the cfe-commits mailing list