Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(406)

Issue 4856049: Reduce duplicate code in pointer comparisons (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 9 months ago by rtrieu
Modified:
12 years, 8 months ago
Reviewers:
Sean Hunt
Base URL:
https://llvm.org/svn/llvm-project/cfe/trunk/
Visibility:
Public.

Description

Committed at 138994.

Patch Set 1 #

Total comments: 6

Patch Set 2 : Fix bad comment and pick a better function name #

Patch Set 3 : Update base file and invert return type for check function. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+84 lines, -72 lines) Patch
M lib/Sema/SemaExpr.cpp View 1 2 7 chunks +84 lines, -72 lines 0 comments Download

Messages

Total messages: 2
Sean Hunt
http://codereview.appspot.com/4856049/diff/1/lib/Sema/SemaExpr.cpp File lib/Sema/SemaExpr.cpp (right): http://codereview.appspot.com/4856049/diff/1/lib/Sema/SemaExpr.cpp#newcode6169 lib/Sema/SemaExpr.cpp:6169: /// \brief Returns true if the member pointers are ...
12 years, 9 months ago (2011-08-09 23:18:40 UTC) #1
rtrieu
12 years, 9 months ago (2011-08-10 21:52:31 UTC) #2
http://codereview.appspot.com/4856049/diff/1/lib/Sema/SemaExpr.cpp
File lib/Sema/SemaExpr.cpp (right):

http://codereview.appspot.com/4856049/diff/1/lib/Sema/SemaExpr.cpp#newcode6169
lib/Sema/SemaExpr.cpp:6169: /// \brief Returns true if the member pointers are
compatible.
On 2011/08/09 23:18:40, Sean Hunt wrote:
> This function doesn't necessarily run on member pointers, right?

Done.

http://codereview.appspot.com/4856049/diff/1/lib/Sema/SemaExpr.cpp#newcode6172
lib/Sema/SemaExpr.cpp:6172: ExprResult &rex) {
On 2011/08/09 23:18:40, Sean Hunt wrote:
> A function named 'check' shouldn't have side effects on its operands. These
> should at least be documented, and it would be good to find a better name.

Done.

http://codereview.appspot.com/4856049/diff/1/lib/Sema/SemaExpr.cpp#newcode6216
lib/Sema/SemaExpr.cpp:6216: static void
diagnoseFunctionPointerToVoidComparison(Sema &S, SourceLocation Loc,
On 2011/08/09 23:18:40, Sean Hunt wrote:
> And this one.

What's wrong with this one?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b