[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-17 Thread Simon Marchi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC337284: [Tooling] Add operator== to CompileCommand (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D49265?vs=155773=155883#toc Repository: rL LLVM

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-17 Thread Simon Marchi via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL337284: [Tooling] Add operator== to CompileCommand (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D49265 Files:

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. Looks good, Thanks! Repository: rC Clang https://reviews.llvm.org/D49265 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread Simon Marchi via Phabricator via cfe-commits
simark updated this revision to Diff 155773. simark added a comment. Add tests for both == and !=. I need to rebuild ~800 files (because I pulled llvm/clang), so I did not actually test it, but I'll do so before pushing tomorrow, of course. Repository: rC Clang

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread Simon Marchi via Phabricator via cfe-commits
simark added a comment. In https://reviews.llvm.org/D49265#1164227, @dblaikie wrote: > In theory you'd need separate tests for op== and op!= returning false > (currently all the tests would pass if both implementations returned true in > all cases), but not the biggest deal. Good point, I'll

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In theory you'd need separate tests for op== and op!= returning false (currently all the tests would pass if both implementations returned true in all cases), but not the biggest deal.

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread Simon Marchi via Phabricator via cfe-commits
simark updated this revision to Diff 155753. simark added a comment. - Add test - Make operator== a function instead of method - Add operator!= (so I can use EXPECT_NE in the test, and because it may be useful in general) Repository: rC Clang https://reviews.llvm.org/D49265 Files:

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread Simon Marchi via Phabricator via cfe-commits
simark added a comment. In https://reviews.llvm.org/D49265#1163740, @dblaikie wrote: > Any chance this can/should be unit tested? (also, in general (though might > not matter in this instance), symmetric operators like == should be > implemented as non-members (though they can still be friends

Re: [PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread David Blaikie via cfe-commits
Any chance this can/should be unit tested? (also, in general (though might not matter in this instance), symmetric operators like == should be implemented as non-members (though they can still be friends and if they are, can be defined inline in the class definition as a member could be), so any

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-16 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Any chance this can/should be unit tested? (also, in general (though might not matter in this instance), symmetric operators like == should be implemented as non-members (though they can still be friends and if they are, can be defined inline in the class definition as a

[PATCH] D49265: [Tooling] Add operator== to CompileCommand

2018-07-12 Thread Simon Marchi via Phabricator via cfe-commits
simark created this revision. Herald added subscribers: cfe-commits, ioeric, ilya-biryukov. It does the obvious thing of comparing all fields. This will be needed for a clangd patch I have in the pipeline. Repository: rC Clang https://reviews.llvm.org/D49265 Files: