Switch token_infos_ to a ValueStore #5633
Conversation
| // For `it->val`, writing `const std::pair` is required; otherwise | ||
| // `mapped_iterator` incorrectly infers the pointer type for `PointerProxy`. | ||
| // NOLINTNEXTLINE(readability-const-return-type) | ||
| auto index_to_id = [&](int32_t i) -> const std::pair<IdT, ConstRefType> { |
There was a problem hiding this comment.
If I'm understanding this correctly, the problem is that the operator-> here is declared const, which means that &R is a const pair* but the return type is non-const pair*. I wonder if upstream would take a patch removing the const there? (And from ReferenceProxy which has the same problem I think.)
In any case, we shouldn't block on fixing this upstream.
There was a problem hiding this comment.
Yes, that sounds right. I wasn't confident that modifying the proxy types to remove the const would be well-received though, because maybe const-ness is important?
toolchain/lex/token_info.h
Outdated
|
|
||
| namespace Carbon::Lex { | ||
|
|
||
| // Storage for the information about a specific token in the buffer. |
There was a problem hiding this comment.
Is it worth mentioning in this comment that this type is an implementation detail of the TokenizedBuffer and Lexer classes and is not public API?
There was a problem hiding this comment.
Done, though it feels a little weird -- I don't think we do that elsewhere, even in something like NumericLiteral
Split out `TokenInfo` to be able to easily write `using ValueType = TokenInfo;` on `TokenIndex`. Also fixes a small type issue on `ValueStore` that affected `mapped_iterator` behavior when writing `old_tokens_it->first < next_offset`.
Split out
TokenInfoto be able to easily writeusing ValueType = TokenInfo;onTokenIndex. Also fixes a small type issue onValueStorethat affectedmapped_iteratorbehavior when writingold_tokens_it->first < next_offset.