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

[Sam McCall via All-commits]

  Branch: refs/heads/release/10.x
  Home:   https://github.com/llvm/llvm-project
  Commit: 41c5efc3f2f22475bf3290309c90e84713511711
      https://github.com/llvm/llvm-project/commit/41c5efc3f2f22475bf3290309c90e84713511711
  Author: Sam McCall <sam.mccall at gmail.com>
  Date:   2020-06-10 (Wed, 10 Jun 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

(cherry picked from commit ec0b9908952a9f4a19c3eb92ba0fc01cffcb8614)


  Commit: 0530e2a811b08f13e8137c29f047ad6bd11967fa
      https://github.com/llvm/llvm-project/commit/0530e2a811b08f13e8137c29f047ad6bd11967fa
  Author: Sam McCall <sam.mccall at gmail.com>
  Date:   2020-06-10 (Wed, 10 Jun 2020)

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

  Log Message:
  -----------
  [Syntax] Merge overlapping top-level macros in TokenBuffer

Summary:
Our previous definition of "top-level" was too informal, and didn't
allow for overlapping macros that each directly produce expanded tokens.
See D77507 for previous discussion.

Fixes http://bugs.llvm.org/show_bug.cgi?id=45428

Reviewers: kadircet, vabridgers

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D77615

(cherry picked from commit d66afd6dde542dc373f87e07fe764c071fe20d76)


Compare: https://github.com/llvm/llvm-project/compare/230b872c290d...0530e2a811b0