[cfe-commits] r101610 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/Sema.h lib/Sema/SemaChecking.cpp test/Sema/builtin-prefetch.c test/Sema/builtin-stackaddress.c test/Sema/builtins.c
Chris Lattner
clattner at apple.com
Sat Apr 17 20:26:08 PDT 2010
On Apr 16, 2010, at 7:26 PM, Eric Christopher wrote:
> Author: echristo
> Date: Fri Apr 16 21:26:23 2010
> New Revision: 101610
>
> URL: http://llvm.org/viewvc/llvm-project?rev=101610&view=rev
> Log:
> Consolidate most of the integer constant expression builtin requirement
> checking into a single function and use that throughout. Remove some
> now unnecessary diagnostics and update tests with now more accurate
> diagnostics.
Thanks for doing this Eric. One stylistic thing: there is no need to say which argument number is problematic, just point to the argument and underline the problematic expression with a source range. Removing the argument # makes it more concise.
IOW, please change "argument 0 to '__builtin_return_address' must be a constant integer" to "argument to '__builtin_return_address' must be a constant integer" or better yet, "argument must be a constant integer" if its obvious what the callee is.
-Chris
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/Sema/builtin-prefetch.c
> cfe/trunk/test/Sema/builtin-stackaddress.c
> cfe/trunk/test/Sema/builtins.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=101610&r1=101609&r2=101610&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Apr 16 21:26:23 2010
> @@ -2854,22 +2854,15 @@
> "%select{too many|too few}0 elements in vector initialization (expected %1 elements, have %2)">;
> def err_altivec_empty_initializer : Error<"expected initializer">;
>
> -def err_stack_const_level : Error<
> - "level argument for a stack address builtin must be constant">;
> -
> -def err_prefetch_invalid_arg_type : Error<
> - "argument to __builtin_prefetch must be of integer type">;
> -def err_prefetch_invalid_arg_ice : Error<
> - "argument to __builtin_prefetch must be a constant integer">;
> def err_argument_invalid_range : Error<
> "argument should be a value from %0 to %1">;
>
> -def err_object_size_invalid_argument : Error<
> - "argument to __builtin_object_size must be a constant integer">;
> -
> def err_builtin_longjmp_invalid_val : Error<
> "argument to __builtin_longjmp must be a constant 1">;
>
> +def err_constant_integer_arg_type : Error<
> + "argument %0 to %1 must be a constant integer">;
> +
> def ext_mixed_decls_code : Extension<
> "ISO C90 forbids mixing declarations and code">;
> def err_non_variable_decl_in_for : Error<
>
> Modified: cfe/trunk/lib/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=101610&r1=101609&r2=101610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.h (original)
> +++ cfe/trunk/lib/Sema/Sema.h Fri Apr 16 21:26:23 2010
> @@ -4352,7 +4352,6 @@
> bool SemaBuiltinVAStart(CallExpr *TheCall);
> bool SemaBuiltinUnorderedCompare(CallExpr *TheCall);
> bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs);
> - bool SemaBuiltinStackAddress(CallExpr *TheCall);
>
> public:
> // Used by C++ template instantiation.
> @@ -4363,7 +4362,8 @@
> bool SemaBuiltinObjectSize(CallExpr *TheCall);
> bool SemaBuiltinLongjmp(CallExpr *TheCall);
> bool SemaBuiltinAtomicOverloaded(CallExpr *TheCall);
> - bool SemaBuiltinEHReturnDataRegNo(CallExpr *TheCall);
> + bool SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
> + llvm::APSInt &Result);
> bool SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
> bool HasVAListArg, unsigned format_idx,
> unsigned firstDataArg);
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=101610&r1=101609&r2=101610&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 16 21:26:23 2010
> @@ -26,6 +26,7 @@
> #include "clang/Lex/Preprocessor.h"
> #include "llvm/ADT/BitVector.h"
> #include "llvm/ADT/STLExtras.h"
> +#include "clang/Basic/TargetBuiltins.h"
> #include <limits>
> using namespace clang;
>
> @@ -155,14 +156,18 @@
> return ExprError();
> break;
> case Builtin::BI__builtin_return_address:
> - case Builtin::BI__builtin_frame_address:
> - if (SemaBuiltinStackAddress(TheCall))
> + case Builtin::BI__builtin_frame_address: {
> + llvm::APSInt Result;
> + if (SemaBuiltinConstantArg(TheCall, 0, Result))
> return ExprError();
> break;
> - case Builtin::BI__builtin_eh_return_data_regno:
> - if (SemaBuiltinEHReturnDataRegNo(TheCall))
> + }
> + case Builtin::BI__builtin_eh_return_data_regno: {
> + llvm::APSInt Result;
> + if (SemaBuiltinConstantArg(TheCall, 0, Result))
> return ExprError();
> break;
> + }
> case Builtin::BI__builtin_shufflevector:
> return SemaBuiltinShuffleVector(TheCall);
> // TheCall will be freed by the smart pointer here, but that's fine, since
> @@ -196,6 +201,15 @@
> if (SemaBuiltinAtomicOverloaded(TheCall))
> return ExprError();
> break;
> +
> + // Target specific builtins start here.
> + case X86::BI__builtin_ia32_palignr128:
> + case X86::BI__builtin_ia32_palignr: {
> + llvm::APSInt Result;
> + if (SemaBuiltinConstantArg(TheCall, 2, Result))
> + return ExprError();
> + break;
> + }
> }
>
> return move(TheCallResult);
> @@ -606,18 +620,6 @@
> return false;
> }
>
> -bool Sema::SemaBuiltinStackAddress(CallExpr *TheCall) {
> - // The signature for these builtins is exact; the only thing we need
> - // to check is that the argument is a constant.
> - SourceLocation Loc;
> - if (!TheCall->getArg(0)->isTypeDependent() &&
> - !TheCall->getArg(0)->isValueDependent() &&
> - !TheCall->getArg(0)->isIntegerConstantExpr(Context, &Loc))
> - return Diag(Loc, diag::err_stack_const_level) << TheCall->getSourceRange();
> -
> - return false;
> -}
> -
> /// SemaBuiltinShuffleVector - Handle __builtin_shufflevector.
> // This is declared to take (...), so we have to check everything.
> Action::OwningExprResult Sema::SemaBuiltinShuffleVector(CallExpr *TheCall) {
> @@ -668,11 +670,9 @@
> TheCall->getArg(i)->isValueDependent())
> continue;
>
> - llvm::APSInt Result(32);
> - if (!TheCall->getArg(i)->isIntegerConstantExpr(Result, Context))
> - return ExprError(Diag(TheCall->getLocStart(),
> - diag::err_shufflevector_nonconstant_argument)
> - << TheCall->getArg(i)->getSourceRange());
> + llvm::APSInt Result;
> + if (SemaBuiltinConstantArg(TheCall, i, Result))
> + return ExprError();
>
> if (Result.getActiveBits() > 64 || Result.getZExtValue() >= numElements*2)
> return ExprError(Diag(TheCall->getLocStart(),
> @@ -709,23 +709,10 @@
> // constant integers.
> for (unsigned i = 1; i != NumArgs; ++i) {
> Expr *Arg = TheCall->getArg(i);
> - if (Arg->isTypeDependent())
> - continue;
> -
> - if (!Arg->getType()->isIntegralType())
> - return Diag(TheCall->getLocStart(), diag::err_prefetch_invalid_arg_type)
> - << Arg->getSourceRange();
> -
> - ImpCastExprToType(Arg, Context.IntTy, CastExpr::CK_IntegralCast);
> - TheCall->setArg(i, Arg);
> -
> - if (Arg->isValueDependent())
> - continue;
> -
> +
> llvm::APSInt Result;
> - if (!Arg->isIntegerConstantExpr(Result, Context))
> - return Diag(TheCall->getLocStart(), diag::err_prefetch_invalid_arg_ice)
> - << SourceRange(Arg->getLocStart(), Arg->getLocEnd());
> + if (SemaBuiltinConstantArg(TheCall, i, Result))
> + return true;
>
> // FIXME: gcc issues a warning and rewrites these to 0. These
> // seems especially odd for the third argument since the default
> @@ -744,42 +731,35 @@
> return false;
> }
>
> -/// SemaBuiltinEHReturnDataRegNo - Handle __builtin_eh_return_data_regno, the
> -/// operand must be an integer constant.
> -bool Sema::SemaBuiltinEHReturnDataRegNo(CallExpr *TheCall) {
> - llvm::APSInt Result;
> - if (!TheCall->getArg(0)->isIntegerConstantExpr(Result, Context))
> - return Diag(TheCall->getLocStart(), diag::err_expr_not_ice)
> - << TheCall->getArg(0)->getSourceRange();
> -
> +/// SemaBuiltinConstantArg - Handle a check if argument ArgNum of CallExpr
> +/// TheCall is a constant expression.
> +bool Sema::SemaBuiltinConstantArg(CallExpr *TheCall, int ArgNum,
> + llvm::APSInt &Result) {
> + Expr *Arg = TheCall->getArg(ArgNum);
> + DeclRefExpr *DRE =cast<DeclRefExpr>(TheCall->getCallee()->IgnoreParenCasts());
> + FunctionDecl *FDecl = cast<FunctionDecl>(DRE->getDecl());
> +
> + if (Arg->isTypeDependent() || Arg->isValueDependent()) return false;
> +
> + if (!Arg->isIntegerConstantExpr(Result, Context))
> + return Diag(TheCall->getLocStart(), diag::err_constant_integer_arg_type)
> + << ArgNum << FDecl->getDeclName() << Arg->getSourceRange();
> +
> return false;
> }
>
> -
> /// SemaBuiltinObjectSize - Handle __builtin_object_size(void *ptr,
> /// int type). This simply type checks that type is one of the defined
> /// constants (0-3).
> // For compatability check 0-3, llvm only handles 0 and 2.
> bool Sema::SemaBuiltinObjectSize(CallExpr *TheCall) {
> - Expr *Arg = TheCall->getArg(1);
> - if (Arg->isTypeDependent())
> - return false;
> -
> - QualType ArgType = Arg->getType();
> - const BuiltinType *BT = ArgType->getAs<BuiltinType>();
> - llvm::APSInt Result(32);
> - if (!BT || BT->getKind() != BuiltinType::Int)
> - return Diag(TheCall->getLocStart(), diag::err_object_size_invalid_argument)
> - << SourceRange(Arg->getLocStart(), Arg->getLocEnd());
> -
> - if (Arg->isValueDependent())
> - return false;
> -
> - if (!Arg->isIntegerConstantExpr(Result, Context)) {
> - return Diag(TheCall->getLocStart(), diag::err_object_size_invalid_argument)
> - << SourceRange(Arg->getLocStart(), Arg->getLocEnd());
> - }
> + llvm::APSInt Result;
> +
> + // Check constant-ness first.
> + if (SemaBuiltinConstantArg(TheCall, 1, Result))
> + return true;
>
> + Expr *Arg = TheCall->getArg(1);
> if (Result.getSExtValue() < 0 || Result.getSExtValue() > 3) {
> return Diag(TheCall->getLocStart(), diag::err_argument_invalid_range)
> << "0" << "3" << SourceRange(Arg->getLocStart(), Arg->getLocEnd());
> @@ -792,11 +772,13 @@
> /// This checks that val is a constant 1.
> bool Sema::SemaBuiltinLongjmp(CallExpr *TheCall) {
> Expr *Arg = TheCall->getArg(1);
> - if (Arg->isTypeDependent() || Arg->isValueDependent())
> - return false;
> + llvm::APSInt Result;
>
> - llvm::APSInt Result(32);
> - if (!Arg->isIntegerConstantExpr(Result, Context) || Result != 1)
> + // TODO: This is less than ideal. Overload this to take a value.
> + if (SemaBuiltinConstantArg(TheCall, 1, Result))
> + return true;
> +
> + if (Result != 1)
> return Diag(TheCall->getLocStart(), diag::err_builtin_longjmp_invalid_val)
> << SourceRange(Arg->getLocStart(), Arg->getLocEnd());
>
>
> Modified: cfe/trunk/test/Sema/builtin-prefetch.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-prefetch.c?rev=101610&r1=101609&r2=101610&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/builtin-prefetch.c (original)
> +++ cfe/trunk/test/Sema/builtin-prefetch.c Fri Apr 16 21:26:23 2010
> @@ -6,8 +6,8 @@
> __builtin_prefetch(&a, 1);
> __builtin_prefetch(&a, 1, 2);
> __builtin_prefetch(&a, 1, 9, 3); // expected-error{{too many arguments to function}}
> - __builtin_prefetch(&a, "hello", 2); // expected-error{{argument to __builtin_prefetch must be of integer type}}
> - __builtin_prefetch(&a, a, 2); // expected-error{{argument to __builtin_prefetch must be a constant integer}}
> + __builtin_prefetch(&a, "hello", 2); // expected-error{{argument 1 to '__builtin_prefetch' must be a constant integer}}
> + __builtin_prefetch(&a, a, 2); // expected-error{{argument 1 to '__builtin_prefetch' must be a constant integer}}
> __builtin_prefetch(&a, 2); // expected-error{{argument should be a value from 0 to 1}}
> __builtin_prefetch(&a, 0, 4); // expected-error{{argument should be a value from 0 to 3}}
> __builtin_prefetch(&a, -1, 4); // expected-error{{argument should be a value from 0 to 1}}
>
> Modified: cfe/trunk/test/Sema/builtin-stackaddress.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-stackaddress.c?rev=101610&r1=101609&r2=101610&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/builtin-stackaddress.c (original)
> +++ cfe/trunk/test/Sema/builtin-stackaddress.c Fri Apr 16 21:26:23 2010
> @@ -4,7 +4,7 @@
> }
>
> void b(unsigned x) {
> -return __builtin_return_address(x); // expected-error{{the level argument for a stack address builtin must be constant}}
> +return __builtin_return_address(x); // expected-error{{argument 0 to '__builtin_return_address' must be a constant integer}}
> }
>
> void* c(unsigned x) {
> @@ -12,5 +12,5 @@
> }
>
> void d(unsigned x) {
> -return __builtin_frame_address(x); // expected-error{{the level argument for a stack address builtin must be constant}}
> +return __builtin_frame_address(x); // expected-error{{argument 0 to '__builtin_frame_address' must be a constant integer}}
> }
>
> Modified: cfe/trunk/test/Sema/builtins.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=101610&r1=101609&r2=101610&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/builtins.c (original)
> +++ cfe/trunk/test/Sema/builtins.c Fri Apr 16 21:26:23 2010
> @@ -60,7 +60,7 @@
> break;
> }
>
> - __builtin_eh_return_data_regno(X); // expected-error {{not an integer constant expression}}
> + __builtin_eh_return_data_regno(X); // expected-error {{argument 0 to '__builtin_eh_return_data_regno' must be a constant integer}}
> }
>
> // PR5062
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list