|
|
Patch Set 1 #
Total comments: 6
Total messages: 2
|
nlewycky1
Looks fine, minor comments. http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp File lib/Sema/SemaChecking.cpp (right): http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcode1831 lib/Sema/SemaChecking.cpp:1831: /// \brief If E is ...
|
14 years, 6 months ago (2011年06月16日 16:53:00 UTC) #1 | |||||||||||||||||||||||||||||
Looks fine, minor comments. http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp File lib/Sema/SemaChecking.cpp (right): http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcod... lib/Sema/SemaChecking.cpp:1831: /// \brief If E is a sizeof expression returns the argument expression, returns *its argument expression http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcod... lib/Sema/SemaChecking.cpp:1837: return SizeOf->getArgumentExpr()->IgnoreParenImpCasts(); Why the IgnoreParenImpCasts()? http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcod... lib/Sema/SemaChecking.cpp:1842: /// \brief If E is a sizeof expression returns the argument type. s/the/its/ again
Thanks for the review! http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp File lib/Sema/SemaChecking.cpp (right): http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcod... lib/Sema/SemaChecking.cpp:1831: /// \brief If E is a sizeof expression returns the argument expression, On 2011年06月16日 16:53:00, nlewycky1 wrote: > returns *its argument expression Done. http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcod... lib/Sema/SemaChecking.cpp:1837: return SizeOf->getArgumentExpr()->IgnoreParenImpCasts(); On 2011年06月16日 16:53:00, nlewycky1 wrote: > Why the IgnoreParenImpCasts()? sizeof ((((x)))) Seemed nice not need every caller to do this. I could potentially not strip imp casts, i can't conceive of a way they would be present in the AST within a sizeof argument (as it would kinda defeat the purpose...) but I was mirroring the logic applied to the src/dest argument in these functions. http://codereview.appspot.com/4641041/diff/1/lib/Sema/SemaChecking.cpp#newcod... lib/Sema/SemaChecking.cpp:1842: /// \brief If E is a sizeof expression returns the argument type. On 2011年06月16日 16:53:00, nlewycky1 wrote: > s/the/its/ again Done.