r198942 - Remove unexpected code completion handling from ConsumeToken()

[Sean Silva]

On Fri, Jan 10, 2014 at 2:35 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 10/01/2014 20:50, Sean Silva wrote:
>
>
>>
>>
>> On Fri, Jan 10, 2014 at 7:37 AM, Alp Toker <alp at nuanti.com <mailto:
>> alp at nuanti.com>> wrote:
>>
>>     Author: alp
>>     Date: Fri Jan 10 08:37:02 2014
>>     New Revision: 198942
>>
>>     URL: http://llvm.org/viewvc/llvm-project?rev=198942&view=rev
>>     Log:
>>     Remove unexpected code completion handling from ConsumeToken()
>>
>>     With this change tok::code_completion is finally handled
>>     exclusively as a
>>     special token kind like other tokens that need special treatment.
>>
>>     All callers have been updated to use the specific token
>>     consumption methods and
>>     the parser has a clear idea the current token isn't special by the
>>     time
>>     ConsumeToken() gets called, so this has been unreachable for some
>>     time.
>>
>>     ConsumeAnyToken() behaviour is unchanged and will continue to support
>>     unexpected code completion as part of the special token path.
>>
>>     This survived an amount of fuzzing and validation, but please ping
>>     the list if
>>     you hit a code path that previously relied on the old unexpected
>>     handler and
>>     now asserts.
>>
>>     Modified:
>>         cfe/trunk/include/clang/Parse/Parser.h
>>
>>     Modified: cfe/trunk/include/clang/Parse/Parser.h
>>     URL:
>>     http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
>> clang/Parse/Parser.h?rev=198942&r1=198941&r2=198942&view=diff
>>     ============================================================
>> ==================
>>     --- cfe/trunk/include/clang/Parse/Parser.h (original)
>>     +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 10 08:37:02 2014
>>     @@ -273,16 +273,12 @@ public:
>>        bool ParseTopLevelDecl(DeclGroupPtrTy &Result);
>>
>>        /// ConsumeToken - Consume the current 'peek token' and lex the
>>     next one.
>>     -  /// This does not work with all kinds of tokens: strings and
>>     specific other
>>     -  /// tokens must be consumed with custom methods below.  This
>>     returns the
>>     -  /// location of the consumed token.
>>     +  /// This does not work with special tokens: string literals,
>>     code completion
>>     +  /// and balanced tokens must be handled using the specific
>>     consume methods.
>>     +  /// Returns the location of the consumed token.
>>        SourceLocation ConsumeToken() {
>>          assert(!isTokenSpecial() &&
>>                 "Should consume special tokens with Consume*Token");
>>     -
>>     -    if (LLVM_UNLIKELY(Tok.is(tok::code_completion)))
>>     -      return handleUnexpectedCodeCompletionToken();
>>
>>
>> Is there some central place where we can mark tok::code_completion as
>> unlikely?
>>
>
> The unexpected code completion tokens that used to end up here are checked
> for precisely now, so the parser will efficiently reach code completion in
> every case without needing to get as far as ConsumeToken().
>
> The checks themselves are of the form Tok.is(tok::code_completion) or
> otherwise exist in switch statements. Perhaps try an LLVM_UNLIKELY() in
> Tok.is() / Tok.isNot()? Not sure if it'd add value -- there are plenty of
> other equally unlikely tokens like tok::eof after all.
>
> To venture a guess, the next lowest hanging fruit might rather be
> ConsumeAnyToken() which has to test and branch for various kinds of special
> token kinds and gets called in tight loops. I don't have numbers and it's
> never showed up on profiles so I'd be still be cautious to spend too much
> time micro-optimizing there.
>

Ok. Sounds like these code sequences will be a good fit for the upcoming
PGO work.

-- Sean Silva


>
> Alp.
>
>
>
>
>> -- Sean Silva
>>
>>     -
>>          PrevTokLocation = Tok.getLocation();
>>          PP.Lex(Tok);
>>          return PrevTokLocation;
>>     @@ -329,7 +325,7 @@ private:
>>        /// isTokenSpecial - True if this token requires special
>>     consumption methods.
>>        bool isTokenSpecial() const {
>>          return isTokenStringLiteral() || isTokenParen() ||
>>     isTokenBracket() ||
>>     -           isTokenBrace();
>>     +           isTokenBrace() || Tok.is(tok::code_completion);
>>        }
>>
>>        /// \brief Returns true if the current token is '=' or is a
>>     type of '='.
>>
>>
>>     _______________________________________________
>>     cfe-commits mailing list
>>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140110/1133c350/attachment.html>