[PATCH] D23387: [Analyzer] Report found fields order in PaddingChecker.

Ben Craig via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 15 11:12:06 PDT 2016


bcraig added a comment.

LGTM


================
Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:217
@@ +216,3 @@
+        // then large field indices to small field indices
+        return std::make_tuple(Align, -Size,
+                               Field ? -static_cast<int>(Field->getFieldIndex())
----------------
alexshap wrote:
> bcraig wrote:
> > Nit:
> > I think std::tie is the more idiomatic way to do this (not that I had it right before either).  You get less copying of values, though that doesn't matter much for the types we are using here.
> std::tie expects an l-value for 2nd argument,
> so it doesn't compile (if i switch to std::tie):
> 
> /Users/alexshap/LLVM/llvm/tools/clang/lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:217:16: error:
>       no matching function for call to 'tie'
>         return std::tie(Align, -Size,
>                ^~~~~~~~
> /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/tuple:814:1: note:
>       candidate function [with _Tp = <const clang::CharUnits, clang::CharUnits,
>       int>] not viable: expects an l-value for 2nd argument
> tie(_Tp&... __t) _NOEXCEPT
> 
Fair enough.  std::make_tuple it is.


https://reviews.llvm.org/D23387





More information about the cfe-commits mailing list