This is an archive of the discontinued LLVM Phabricator instance.

[clang-diff] Fix the html output for CXXOperatorCallExpr
AcceptedPublic

Authored by johannes on Aug 22 2017, 2:17 AM.

Details

Reviewers
arphaman
Summary

The operator is always the first child of such an expression.
If it is an infix operator, we want to print the LHS first.

Event Timeline

johannes created this revision.Aug 22 2017, 2:17 AM
arphaman added inline comments.Aug 23 2017, 8:30 AM
test/Tooling/clang-diff-ast.cpp
42

Looks like an unrelated change.

113

This test doesn't work because it passes prior to this patch. You should test the HTML output instead.

johannes updated this revision to Diff 112416.Aug 23 2017, 12:02 PM

provide a test
need to update the maxsize parameter to -s=200 here because the test file is
expected to fit within this size

arphaman accepted this revision.Aug 24 2017, 6:33 AM

LGTM with one request below:

tools/clang-diff/ClangDiff.cpp
319

Please add a short comment that describes why this out-of-order traversal is required

This revision is now accepted and ready to land.Aug 24 2017, 6:33 AM
johannes added inline comments.Aug 25 2017, 8:39 AM
tools/clang-diff/ClangDiff.cpp
319

Should we do this in LexicallyOrderedRecursiveASTVisitor?

There are some other cases with CXXOperatorCallExpr where the order needs to be changed, e.g. postfix operators, operator->, operator() and operator[].
It can be done by sorting by SourceLocation of the first two elements, as the operator is always the first one.

arphaman added inline comments.Aug 25 2017, 8:41 AM
tools/clang-diff/ClangDiff.cpp
319

Sure, that would make sense. You probably don't need to sort, just figure out the right order from the specific combination of operator and infix/postfix.

shafik added a subscriber: shafik.Sep 21 2023, 9:14 AM

Is this relevant anymore or should we close it?

Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2023, 9:14 AM