[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-11-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 2 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:447 +tryConsumeSpelledUntil(File, EndOffset + 1, SpelledIndex).hasValue(); +(void)HitMapping; +assert(!HitMapping && "recursive

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-11-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:447 +tryConsumeSpelledUntil(File, EndOffset + 1, SpelledIndex).hasValue(); +(void)HitMapping; +assert(!HitMapping && "recursive macro expansion?"); What is

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-24 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL364236: [Syntax] Do not glue multiple empty PP expansions to a single mapping (authored by ibiryukov, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:319 + /// 2. macro name and arguments for macro expansions. + using PPExpansions = llvm::DenseMap; class Builder; sammccall wrote: > do I understand right that

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-24 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 206246. ilya-biryukov marked 10 inline comments as done. ilya-biryukov added a comment. - Address comments, document code. - s/Expansion/CollectedExpansions. - Added FIXMEs for macro arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-24 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:315 private: + /// Maps from a start location to an end location of transformations performed + /// by

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. To clarify, I don't think there are new concepts in this patch. Previously, we only took information from source locations into account when building token buffers. That works fine in most cases, but not enough to find the boundaries of empty macro expansions. In

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 205537. ilya-biryukov marked 3 inline comments as done. ilya-biryukov added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62953/new/ https://reviews.llvm.org/D62953 Files:

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked 7 inline comments as done. ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:318 + /// 2. macro name and arguments for macro expansions. + using SpelledMappings = + llvm::DenseMap;

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-19 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Sorry, I'm having trouble understanding this patch. Can you try to find some clearer names for the new concepts, or describe how they differ? Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:318 + /// 2. macro name and arguments for macro

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment. In D62953#1547799 , @sammccall wrote: > Can you explain why this is important? > (in the code) I've added a few comments into the code that builds token buffers, but I couldn't figure out a good place to mention this in

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-18 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 205377. ilya-biryukov added a comment. - Added comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62953/new/ https://reviews.llvm.org/D62953 Files: clang/include/clang/Tooling/Syntax/Tokens.h

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-18 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Can you explain why this is important? (in the code) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62953/new/ https://reviews.llvm.org/D62953 ___ cfe-commits mailing list

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-06-06 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a project: clang. This change makes sure we have a single mapping for each macro expansion, even if the result of expansion was empty. To achieve that, we take information from PPCallbacks::MacroExpands