On Thu, Sep 6, 2018 at 10:05 PM, Stephen Kelly via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > Yes, this was a prerequisite to https://reviews.llvm.org/D51259. For some > reason the reviewer accepted that one, but not this one. I simply assumed > that was a mistake and committed this one. I'm not sure this is how the review works in LLVM, in general.
> Perhaps part of the problem is that I do one thing per commit, and that > confuses reviewers. Maybe I should squash things for review? I don't know. You can specify the dependencies between the differentials in phabricator. > Thanks for any post-commit review! > > Stephen. Roman. > On 31/08/18 07:06, Roman Lebedev via cfe-commits wrote: >> >> I don't think this was reviewed. The differential is not in 'accepted' >> state. >> >> Roman. >> >> On Fri, Aug 31, 2018 at 2:11 AM, Stephen Kelly via cfe-commits >> <cfe-commits@lists.llvm.org> wrote: >>> >>> Author: steveire >>> Date: Thu Aug 30 16:11:01 2018 >>> New Revision: 341141 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=341141&view=rev >>> Log: >>> Extract parseBindID method >>> >>> Subscribers: cfe-commits >>> >>> Differential Revision: https://reviews.llvm.org/D51258 >>> >>> Modified: >>> cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h >>> cfe/trunk/lib/ASTMatchers/Dynamic/Parser.cpp >>> >>> Modified: cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h?rev=341141&r1=341140&r2=341141&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h (original) >>> +++ cfe/trunk/include/clang/ASTMatchers/Dynamic/Parser.h Thu Aug 30 >>> 16:11:01 2018 >>> @@ -234,6 +234,7 @@ private: >>> const NamedValueMap *NamedValues, >>> Diagnostics *Error); >>> >>> + bool parseBindID(std::string &BindID); >>> bool parseExpressionImpl(VariantValue *Value); >>> bool parseMatcherExpressionImpl(const TokenInfo &NameToken, >>> VariantValue *Value); >>> >>> Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Parser.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Parser.cpp?rev=341141&r1=341140&r2=341141&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/ASTMatchers/Dynamic/Parser.cpp (original) >>> +++ cfe/trunk/lib/ASTMatchers/Dynamic/Parser.cpp Thu Aug 30 16:11:01 2018 >>> @@ -359,6 +359,43 @@ bool Parser::parseIdentifierPrefixImpl(V >>> return parseMatcherExpressionImpl(NameToken, Value); >>> } >>> >>> +bool Parser::parseBindID(std::string &BindID) { >>> + // Parse .bind("foo") >>> + assert(Tokenizer->peekNextToken().Kind == TokenInfo::TK_Period); >>> + Tokenizer->consumeNextToken(); // consume the period. >>> + const TokenInfo BindToken = Tokenizer->consumeNextToken(); >>> + if (BindToken.Kind == TokenInfo::TK_CodeCompletion) { >>> + addCompletion(BindToken, MatcherCompletion("bind(\"", "bind", 1)); >>> + return false; >>> + } >>> + >>> + const TokenInfo OpenToken = Tokenizer->consumeNextToken(); >>> + const TokenInfo IDToken = Tokenizer->consumeNextToken(); >>> + const TokenInfo CloseToken = Tokenizer->consumeNextToken(); >>> + >>> + // TODO: We could use different error codes for each/some to be more >>> + // explicit about the syntax error. >>> + if (BindToken.Kind != TokenInfo::TK_Ident || >>> + BindToken.Text != TokenInfo::ID_Bind) { >>> + Error->addError(BindToken.Range, Error->ET_ParserMalformedBindExpr); >>> + return false; >>> + } >>> + if (OpenToken.Kind != TokenInfo::TK_OpenParen) { >>> + Error->addError(OpenToken.Range, Error->ET_ParserMalformedBindExpr); >>> + return false; >>> + } >>> + if (IDToken.Kind != TokenInfo::TK_Literal || >>> !IDToken.Value.isString()) { >>> + Error->addError(IDToken.Range, Error->ET_ParserMalformedBindExpr); >>> + return false; >>> + } >>> + if (CloseToken.Kind != TokenInfo::TK_CloseParen) { >>> + Error->addError(CloseToken.Range, >>> Error->ET_ParserMalformedBindExpr); >>> + return false; >>> + } >>> + BindID = IDToken.Value.getString(); >>> + return true; >>> +} >>> + >>> /// Parse and validate a matcher expression. >>> /// \return \c true on success, in which case \c Value has the matcher >>> parsed. >>> /// If the input is malformed, or some argument has an error, it >>> @@ -425,38 +462,8 @@ bool Parser::parseMatcherExpressionImpl( >>> >>> std::string BindID; >>> if (Tokenizer->peekNextToken().Kind == TokenInfo::TK_Period) { >>> - // Parse .bind("foo") >>> - Tokenizer->consumeNextToken(); // consume the period. >>> - const TokenInfo BindToken = Tokenizer->consumeNextToken(); >>> - if (BindToken.Kind == TokenInfo::TK_CodeCompletion) { >>> - addCompletion(BindToken, MatcherCompletion("bind(\"", "bind", 1)); >>> + if (!parseBindID(BindID)) >>> return false; >>> - } >>> - >>> - const TokenInfo OpenToken = Tokenizer->consumeNextToken(); >>> - const TokenInfo IDToken = Tokenizer->consumeNextToken(); >>> - const TokenInfo CloseToken = Tokenizer->consumeNextToken(); >>> - >>> - // TODO: We could use different error codes for each/some to be more >>> - // explicit about the syntax error. >>> - if (BindToken.Kind != TokenInfo::TK_Ident || >>> - BindToken.Text != TokenInfo::ID_Bind) { >>> - Error->addError(BindToken.Range, >>> Error->ET_ParserMalformedBindExpr); >>> - return false; >>> - } >>> - if (OpenToken.Kind != TokenInfo::TK_OpenParen) { >>> - Error->addError(OpenToken.Range, >>> Error->ET_ParserMalformedBindExpr); >>> - return false; >>> - } >>> - if (IDToken.Kind != TokenInfo::TK_Literal || >>> !IDToken.Value.isString()) { >>> - Error->addError(IDToken.Range, Error->ET_ParserMalformedBindExpr); >>> - return false; >>> - } >>> - if (CloseToken.Kind != TokenInfo::TK_CloseParen) { >>> - Error->addError(CloseToken.Range, >>> Error->ET_ParserMalformedBindExpr); >>> - return false; >>> - } >>> - BindID = IDToken.Value.getString(); >>> } >>> >>> if (!Ctor) >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
