<div dir="ltr">If all the call sites were in SemaChecking, we could just call the static function to handle everything. However, with them spread across two files, a forwarding function in Sema is needed. Since the Sema function is only used in one place and is not generally useful outside of Sema, it is better to make it private so it is not exposed outside of Sema.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 17, 2014 at 9:43 AM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks. I felt like we should consistently call the Sema:: variety instead of the helper function in some places. So,<br>
I was forced to make it public.<br>
<br>
- Fariborz<br>
<div class="HOEnZb"><div class="h5"><br>
> On Nov 14, 2014, at 10:37 PM, Richard Trieu <<a href="mailto:rtrieu@google.com">rtrieu@google.com</a>> wrote:<br>
><br>
> Author: rtrieu<br>
> Date: Sat Nov 15 00:37:39 2014<br>
> New Revision: 222081<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=222081&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=222081&view=rev</a><br>
> Log:<br>
> Fix issues missed during the review of r222099.<br>
><br>
> Shift some functions around, make a method in Sema private,<br>
> call the correct overloaded function. No functional change.<br>
><br>
> Modified:<br>
> cfe/trunk/include/clang/Sema/Sema.h<br>
> cfe/trunk/lib/Sema/SemaChecking.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Sema/Sema.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=222081&r1=222080&r2=222081&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=222081&r1=222080&r2=222081&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Sema/Sema.h (original)<br>
> +++ cfe/trunk/include/clang/Sema/Sema.h Sat Nov 15 00:37:39 2014<br>
> @@ -2771,8 +2771,6 @@ public:<br>
> const AttributeList *AttrList);<br>
><br>
> void checkUnusedDeclAttributes(Declarator &D);<br>
> -<br>
> - void CheckBoolLikeConversion(Expr *E, SourceLocation CC);<br>
><br>
> /// Determine if type T is a valid subject for a nonnull and similar<br>
> /// attributes. By default, we look through references (the behavior used by<br>
> @@ -8592,6 +8590,7 @@ private:<br>
><br>
> void CheckFloatComparison(SourceLocation Loc, Expr* LHS, Expr* RHS);<br>
> void CheckImplicitConversions(Expr *E, SourceLocation CC = SourceLocation());<br>
> + void CheckBoolLikeConversion(Expr *E, SourceLocation CC);<br>
> void CheckForIntOverflow(Expr *E);<br>
> void CheckUnsequencedOperations(Expr *E);<br>
><br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=222081&r1=222080&r2=222081&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=222081&r1=222080&r2=222081&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sat Nov 15 00:37:39 2014<br>
> @@ -6526,6 +6526,14 @@ void CheckConditionalOperator(Sema &S, C<br>
> E->getType(), CC, &Suspicious);<br>
> }<br>
><br>
> +/// CheckBoolLikeConversion - Check conversion of given expression to boolean.<br>
> +/// Input argument E is a logical expression.<br>
> +static void CheckBoolLikeConversion(Sema &S, Expr *E, SourceLocation CC) {<br>
> + if (S.getLangOpts().Bool)<br>
> + return;<br>
> + CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC);<br>
> +}<br>
> +<br>
> /// AnalyzeImplicitConversions - Find and report any interesting<br>
> /// implicit conversions in the given expression. There are a couple<br>
> /// of competing diagnostics here, -Wconversion and -Wsign-compare.<br>
> @@ -6606,12 +6614,12 @@ void AnalyzeImplicitConversions(Sema &S,<br>
> AnalyzeImplicitConversions(S, ChildExpr, CC);<br>
> }<br>
> if (BO && BO->isLogicalOp()) {<br>
> - S.CheckBoolLikeConversion(BO->getLHS(), BO->getLHS()->getExprLoc());<br>
> - S.CheckBoolLikeConversion(BO->getRHS(), BO->getRHS()->getExprLoc());<br>
> + ::CheckBoolLikeConversion(S, BO->getLHS(), BO->getLHS()->getExprLoc());<br>
> + ::CheckBoolLikeConversion(S, BO->getRHS(), BO->getRHS()->getExprLoc());<br>
> }<br>
> if (const UnaryOperator *U = dyn_cast<UnaryOperator>(E))<br>
> if (U->getOpcode() == UO_LNot)<br>
> - S.CheckBoolLikeConversion(U->getSubExpr(), CC);<br>
> + ::CheckBoolLikeConversion(S, U->getSubExpr(), CC);<br>
> }<br>
><br>
> } // end anonymous namespace<br>
> @@ -6670,18 +6678,6 @@ static bool IsInAnyMacroBody(const Sourc<br>
> return false;<br>
> }<br>
><br>
> -/// CheckBoolLikeConversion - Check conversion of given expression to boolean.<br>
> -/// Input argument E is a logical expression.<br>
> -static void CheckBoolLikeConversion(Sema &S, Expr *E, SourceLocation CC) {<br>
> - if (S.getLangOpts().Bool)<br>
> - return;<br>
> - CheckImplicitConversion(S, E->IgnoreParenImpCasts(), S.Context.BoolTy, CC);<br>
> -}<br>
> -<br>
> -void Sema::CheckBoolLikeConversion(Expr *E, SourceLocation CC) {<br>
> - ::CheckBoolLikeConversion(*this, E, CC);<br>
> -}<br>
> -<br>
> /// \brief Diagnose pointers that are always non-null.<br>
> /// \param E the expression containing the pointer<br>
> /// \param NullKind NPCK_NotNull if E is a cast to bool, otherwise, E is<br>
> @@ -6839,6 +6835,12 @@ void Sema::CheckImplicitConversions(Expr<br>
> AnalyzeImplicitConversions(*this, E, CC);<br>
> }<br>
><br>
> +/// CheckBoolLikeConversion - Check conversion of given expression to boolean.<br>
> +/// Input argument E is a logical expression.<br>
> +void Sema::CheckBoolLikeConversion(Expr *E, SourceLocation CC) {<br>
> + ::CheckBoolLikeConversion(*this, E, CC);<br>
> +}<br>
> +<br>
> /// Diagnose when expression is an integer constant expression and its evaluation<br>
> /// results in integer overflow<br>
> void Sema::CheckForIntOverflow (Expr *E) {<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
</div></div></blockquote></div><br></div>