This is an archive of the discontinued LLVM Phabricator instance.

refactor/cleanup ClangExpressionParser::Parse
ClosedPublic

Authored by ldrumm on Feb 15 2016, 11:37 AM.

Details

Summary
  • fix return type: ClangExpressionParser::Parse returns unsigned, but was actually returning a signed value, num_errors.
  • use helper clang::TextDiagnosticBuffer::getNumErrors() instead of counting the errors ourself.
  • limit scoping of block-level automatic variables as much as practical.
  • remove reused multipurpose TextDiagnosticBuffer::const_iterator in favour of loop-scoped err, warn, and note variables in the diagnostic printing code.
  • refactor diagnostic printing loops to use a proper loop invariant.

Diff Detail

Event Timeline

ldrumm updated this revision to Diff 48002.Feb 15 2016, 11:37 AM
ldrumm retitled this revision from to refactor/cleanup ClangExpressionParser::Parse.
ldrumm updated this object.
ldrumm added reviewers: clayborg, zturner, spyffe.
ldrumm added a subscriber: lldb-commits.
clayborg resigned from this revision.Feb 18 2016, 4:36 PM
clayborg removed a reviewer: clayborg.

I will let Sean Callanan OK this one.

spyffe accepted this revision.Feb 18 2016, 5:06 PM
spyffe edited edge metadata.

Aside from a minor nit, this looks like good cleanup. If it passes the testsuite, feel free to commit after addressing the inline comment.

source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
522

I'm okay with use of auto when the type is really onerous to express in code, but in this case TextDiagnosticBuffer::const_iterator is pretty compact, and makes the type of warn more obvious. Could we use that instead of auto?

This revision is now accepted and ready to land.Feb 18 2016, 5:06 PM
This revision was automatically updated to reflect the committed changes.