[all-commits] [llvm/llvm-project] ec0b99: [Syntax] Simplify TokenCollector::Builder, use cap...

[Sam McCall via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: ec0b9908952a9f4a19c3eb92ba0fc01cffcb8614
      https://github.com/llvm/llvm-project/commit/ec0b9908952a9f4a19c3eb92ba0fc01cffcb8614
  Author: Sam McCall <sam.mccall at gmail.com>
  Date:   2020-04-07 (Tue, 07 Apr 2020)

  Changed paths:
    M clang/lib/Tooling/Syntax/Tokens.cpp

  Log Message:
  -----------
  [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