r198942 - Remove unexpected code completion handling from ConsumeToken()

[Alp Toker]

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.

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