Page MenuHomePhabricator

[Syntax] Add mapping from spelled to expanded tokens for TokenBuffer
AcceptedPublic

Authored by ilya-biryukov on Sun, Jan 12, 9:16 PM.

Details

Reviewers
sammccall
Summary

Same restrictions apply as in the other direction: macro arguments are
not supported yet, only full macro expansions can be mapped.

Diff Detail

Event Timeline

ilya-biryukov created this revision.Sun, Jan 12, 9:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Jan 12, 9:16 PM

Unit tests: pass. 61746 tests passed, 0 failed and 780 were skipped.

clang-tidy: fail. Please fix clang-tidy findings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

sammccall accepted this revision.Tue, Jan 21, 3:44 AM
sammccall added inline comments.
clang/include/clang/Tooling/Syntax/Tokens.h
228

Nit: move the FIXME up here? Documenting this justifies the signature, but currently it *never* happens.

248

out of curiosity, do you have a motivating use for this function?

I think it's clear enough that it will be useful, completeness dictates it should be here, and use cases aren't always the best way to think about designing these libraries. But it'd help me understand some of the details better.

clang/lib/Tooling/Syntax/Tokens.cpp
198

This is a little surprising (vs returning {}).

It seems plausible that you'll map file offsets -> spelled tokens -> expanded tokens, and that the middle step might "accidentally" be empty. Caller can always check, but why require them to here?

205

nit: mind spelling this type out? it's important, not particularly verbose, and long-lived

209

This section could use some comments about how the index calculations relate to high-level concepts.

AIUI the branches are: spelled token precedes all PP usage, spelled token is transformed by PP (subcases: macro args and other PP usage), spelled token follows PP usage.

The next section probably only needs to comment about the purpose of the divergence (+1/End to refer to one-past the region of interest).

This revision is now accepted and ready to land.Tue, Jan 21, 3:44 AM