[Syntax] Simplify TokenCollector::Builder, use captured expansion bounds. NFC
Summary: The motivation here is fixing https://bugs.llvm.org/show_bug.cgi?id=45428, see D77507. The fundamental problem is that a "top-level" expansion wasn't precisely defined. Repairing this concept means that TokenBuffer's "top-level expansion" may not correspond to a single macro expansion. Example: ``` M(2); // expands to 1+2 ``` The expansions overlap, but neither expansion alone yields all the tokens. We need a TokenBuffer::Mapping that corresponds to their union. This is fairly easy to fix in CollectPPExpansions, but the current design of TokenCollector::Builder needs a fix too as it relies on the macro's expansion range rather than the captured expansion bounds. This fix is hard to make due to the way code is reused within Builder. And honestly, I found that code pretty hard to reason about too. The new approach doesn't use the expansion range, but only the expansion location: it assumes an expansion is the contiguous set of expanded tokens with the same expansion location, which seems like a reasonable formalization of the "top-level" notion. And hopefully the control flow is easier to follow too, it's considerably shorter even with more documentation. Reviewers: kadircet Subscribers: cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D77614
Loading
Please sign in to comment