[cfe-commits] [patch] Added a suggestion when a std::string is passed to printf()
Richard Smith
richard at metafoo.co.uk
Wed Jun 13 16:09:39 PDT 2012
On Wed, Jun 13, 2012 at 2:14 PM, Sam Panzer <panzer at google.com> wrote:
> Ping.
>
> On Wed, Jun 6, 2012 at 6:35 PM, Sam Panzer <panzer at google.com> wrote:
>
>> Thanks again for the input.
>>
>> Of the two main issues,
>> (1) the POD_Type enum
>> I used the enum to distinguish between three cases: one requires a
>> warning for C++98 compatibility, one requires the standard
>> can't-pass-non-pod-to-vararg warning, and the third does nothing. Since the
>> code that checks for non-POD arguments to vararg functions lives in two
>> places because of the way format-string checking is implemented, I thought
>> it made sense to separate the pod-kind-determining logic from the
>> warning-emitting logic.
>>
>> Does someone have a suggestion for implementing this? I can think of two
>> other approaches, and neither seems as clean: levelOfTriviality could
>> either take an extra argument that tells it not to emit warnings, or it
>> could take a series of callbacks for the three cases.
>>
>> (2) It is possible for SemaChecking to reuse the logic from
>> hasFormatStringArgument(), since both of them really compute the (expected)
>> location of the format string. I refactored a bit to share the relevant
>> logic, though there isn't too much in common. Is this an improvement?
>>
>> On Wed, Jun 6, 2012 at 5:20 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>
>>> Some minor comments, and I'll let others start chiming in here to make
>>> sure there is general agreement about the approach:
>>>
>>>
>>> +def warn_non_pod_vararg_with_format_string : Warning<
>>> + "cannot pass non-POD object of type %0 to variadic function; "
>>> + "expected type was %1">,
>>>
>>> I'd like to mention that the expectation stems from a format string here:
>>>
>>> '... variadic function; the format string expects it to have type %1'
>>>
>>> or some such...
>>>
>>> + // Used for determining in which context a type is considered POD
>>> + enum PODType {
>>> + POD,
>>> + CXX11_POD,
>>> + ObjC_Lifetime_POD,
>>> + NON_POD
>>> + };
>>>
>>> The LLVM coding conventions on enums are much more precise than other
>>> things. I would vaguely expect it to look like:
>>>
>>> enum PODKind {
>>> PK_NonPOD,
>>> PK_POD,
>>> PK_CXX11POD,
>>> PK_ObjCLifetimePOD
>>> };
>>>
>>> +
>>> + // Determines which PODType fits an expression.
>>> + PODType levelOfTriviality(const Expr *E);
>>> +
>>>
>>> Actually, looking at how this function is implemented, I think you're
>>> going about it the wrong way. This isn't a generalizable concept, it is a
>>> very specific and precise query: isValidTypeForVarargArgument or some such.
>>> It should return true or false, no enum required.
>>>
>>
>>> + bool HasformatString =
>>> false);
>>>
>>> s/HasformatString/HasFormatString/
>>>
>>> +// Given a variadic function, decides whether it looks like we have a
>>> +// printf-style format string and arguments)
>>> +bool Sema::hasFormatStringArgument(FunctionDecl *FDecl,
>>> + Expr **Args, unsigned NumArgs) {
>>>
>>> I don't see you using this function from anywhere but new code -- is
>>> there a reason that SemaChecking can't use this logic?
>>>
>>> + // Decide if we're probably inspecting a printf-like function
>>> +
>>> + bool HasFormatString = hasFormatStringArgument(FDecl, Args,
>>> NumArgs);
>>>
>>> I think the comment is now extraneous.
>>>
>>>
>>> On Wed, Jun 6, 2012 at 4:56 PM, Sam Panzer <panzer at google.com> wrote:
>>>
>>>> Parens removed, though operator-> is unavailable here. For future
>>>> reference, is Clang averse to extra variable declarations in favor of extra
>>>> method calls?
>>>
>>>
>>>
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td
b/include/clang/Basic/DiagnosticSemaKinds.td
index 3dc03fa..c51302c 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4681,6 +4681,11 @@ def err_cannot_pass_objc_interface_to_vararg : Error<
"cannot pass object with interface type %0 by-value through variadic "
"%select{function|block|method}1">;
+def warn_non_pod_vararg_with_format_string : Warning<
+ "cannot pass non-POD object of type %0 to variadic function; "
In C++11, this should be 'non-trivial' not 'non-POD'.
+ "expected type from format string was %1">,
+ InGroup<DiagGroup<"non-pod-varargs">>, DefaultError;
+
def warn_cannot_pass_non_pod_arg_to_vararg : Warning<
"cannot pass object of %select{non-POD|non-trivial}0 type %1
through variadic"
" %select{function|block|method|constructor}2; call will abort at runtime">,
@@ -5161,6 +5166,7 @@ def warn_scanf_scanlist_incomplete : Warning<
"no closing ']' for '%%[' in scanf format string">,
InGroup<Format>;
def note_format_string_defined : Note<"format string is defined here">;
+def note_printf_c_str: Note<"did you mean to call the %0 method?">;
def warn_null_arg : Warning<
"null passed to a callee which requires a non-null argument">,
@@ -5617,4 +5623,3 @@ def err_module_private_definition : Error<
}
} // end of sema component.
-
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index f0d3213..c1d2933 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -6395,6 +6395,17 @@ public:
VariadicDoesNotApply
};
+ // Used for determining in which context a type is considered POD
+ enum PODKind {
+ PK_POD,
+ PK_CXX11_POD,
+ PK_ObjC_Lifetime_POD,
+ PK_NON_POD
+ };
+
+ // Determines which PODKind fits an expression.
+ PODKind levelOfTriviality(const Expr *E);
This seems to be specific to vararg arguments, so I'd prefer for it to
be phrased in those terms. Possibly VarArgKind with values
VAK_Invalid, VAK_Valid, and VAK_ValidInCXX11 (for the C++98 compat
warning), and isValidVarArgType?
I would also suggest changing the argument type from const Expr * to
QualType, since only the type is relevant.
+
/// GatherArgumentsForCall - Collector argument expressions for various
/// form of call prototypes.
bool GatherArgumentsForCall(SourceLocation CallLoc,
@@ -6409,7 +6420,8 @@ public:
// DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but
// will warn if the resulting type is not a POD type.
ExprResult DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
- FunctionDecl *FDecl);
+ FunctionDecl *FDecl,
+ bool HasFormatString = false);
// UsualArithmeticConversions - performs the UsualUnaryConversions on it's
// operands and then handles various conversions that are common to binary
@@ -6985,6 +6997,19 @@ private:
const ArraySubscriptExpr *ASE=0,
bool AllowOnePastEnd=true, bool IndexNegated=false);
void CheckArrayAccess(const Expr *E);
+ bool hasFormatStringArgument(FunctionDecl *FDecl, Expr **Args,
+ unsigned NumArgs);
+ // Used to grab the relevant information from a FormatAttr and a
+ // FunctionDeclaration.
+ struct FormatStringInfo {
+ unsigned format_idx;
+ unsigned firstDataArg;
+ bool hasVAListArg;
Even though these are just being factored out from elsewhere, please
fix the names to conform to the coding standard: FormatIdx,
FirstDataArg, HasVAListArg.
+ };
+
+ bool getFormatStringInfo(const FormatAttr *Format, Expr **Args,
+ unsigned NumArgs, bool IsCXXMember,
+ FormatStringInfo *FSI);
bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall);
bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc,
Expr **Args, unsigned NumArgs);
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index e818f5f..b075331 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -16,6 +16,7 @@
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/Initialization.h"
+#include "clang/Sema/Lookup.h"
#include "clang/Sema/ScopeInfo.h"
#include "clang/Analysis/Analyses/FormatString.h"
#include "clang/AST/ASTContext.h"
@@ -418,6 +419,49 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned
BuiltinID, CallExpr *TheCall) {
return false;
}
+
+// Given a variadic function, decides whether it looks like we have a
+// printf-style format string and arguments)
Please add a '.', and use '///' so doxygen picks this comment up.
+bool Sema::hasFormatStringArgument(FunctionDecl *FDecl, Expr **Args,
+ unsigned NumArgs) {
+ // Anonymous and lambda functions will be assumed not to be format strings
The !FDecl case will mainly be calls through function pointers. For a
lambda, we should actually have an FDecl. Indeed, g++ supports format
attributes in such a case:
auto f = [](const char *p, ...) __attribute__((format(printf, 2, 3)))
{ return 0; };
int k = f("%d", 10.2); // warning in gcc
+ if (!FDecl)
+ return false;
+
+ FormatStringInfo FSI;
+ for (specific_attr_iterator<FormatAttr>
+ I = FDecl->specific_attr_begin<FormatAttr>(),
+ E = FDecl->specific_attr_end<FormatAttr>(); I != E ; ++I) {
+ if (getFormatStringInfo(*I, Args, NumArgs,
isa<CXXMethodDecl>(FDecl), &FSI))
+ return true;
+ }
+ return false;
+}
+
+// If so, fills in the FormatStringInfo parameter to have the correct
format_idx
+// and firstDataArg.
Please reword the 'If so' so this comment stands on its own, and
explain what the return value means.
+bool Sema::getFormatStringInfo(const FormatAttr *Format, Expr **Args,
+ unsigned NumArgs, bool IsCXXMember,
+ FormatStringInfo *FSI) {
+ // The way the format attribute works in GCC, the implicit this argument
+ // of member functions is counted. However, it doesn't appear in our own
+ // lists, so decrement format_idx in that case.
This comment should presumably be attached to the 'if (IsCXXMember)' below.
+ FSI->hasVAListArg = Format->getFirstArg() == 0;
+ FSI->format_idx = Format->getFormatIdx() - 1;
+ FSI->firstDataArg = FSI->hasVAListArg ? 0 : Format->getFirstArg() - 1;
+
+ // We need to adjust the index of the format parameter because
+ // the `this` parameter is not counted in the argument list
+ if (IsCXXMember) {
+ if(FSI->format_idx == 0)
+ return false;
+ --FSI->format_idx;
+ if (FSI->firstDataArg != 0)
+ --FSI->firstDataArg;
+ }
+ return true;
+}
+
/// CheckFunctionCall - Check a direct function call for various correctness
/// and safety properties not strictly enforced by the C type system.
bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
@@ -1702,9 +1746,6 @@ Sema::FormatStringType
Sema::GetFormatStringType(const FormatAttr *Format) {
/// functions) for correct use of format strings.
void Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) {
bool IsCXXMember = false;
- // The way the format attribute works in GCC, the implicit this argument
- // of member functions is counted. However, it doesn't appear in our own
- // lists, so decrement format_idx in that case.
IsCXXMember = isa<CXXMemberCallExpr>(TheCall);
CheckFormatArguments(Format, TheCall->getArgs(), TheCall->getNumArgs(),
IsCXXMember, TheCall->getRParenLoc(),
@@ -1714,18 +1755,12 @@ void Sema::CheckFormatArguments(const
FormatAttr *Format, CallExpr *TheCall) {
void Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args,
unsigned NumArgs, bool IsCXXMember,
SourceLocation Loc, SourceRange Range) {
- bool HasVAListArg = Format->getFirstArg() == 0;
- unsigned format_idx = Format->getFormatIdx() - 1;
- unsigned firstDataArg = HasVAListArg ? 0 : Format->getFirstArg() - 1;
- if (IsCXXMember) {
- if (format_idx == 0)
- return;
- --format_idx;
- if(firstDataArg != 0)
- --firstDataArg;
+ FormatStringInfo FSI;
+ if (getFormatStringInfo(Format, Args, NumArgs, IsCXXMember, &FSI)) {
+ CheckFormatArguments(Args, NumArgs, FSI.hasVAListArg, FSI.format_idx,
+ FSI.firstDataArg, GetFormatStringType(Format), Loc,
+ Range);
}
- CheckFormatArguments(Args, NumArgs, HasVAListArg, format_idx,
- firstDataArg, GetFormatStringType(Format), Loc, Range);
}
void Sema::CheckFormatArguments(Expr **Args, unsigned NumArgs,
@@ -2138,6 +2173,10 @@ public:
bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
const char *startSpecifier,
unsigned specifierLen);
+ bool checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
+ const char *StartSpecifier,
+ unsigned SpecifierLen,
+ const Expr *E);
bool HandleAmount(const analyze_format_string::OptionalAmount &Amt,
unsigned k,
const char *startSpecifier, unsigned specifierLen);
@@ -2152,6 +2191,9 @@ public:
const analyze_printf::OptionalFlag &ignoredFlag,
const analyze_printf::OptionalFlag &flag,
const char *startSpecifier, unsigned specifierLen);
+ bool checkForCStrMembers(const analyze_printf::ArgTypeResult &ATR,
+ const Expr *E, const CharSourceRange &CSR);
+
};
}
@@ -2269,6 +2311,66 @@ void CheckPrintfHandler::HandleIgnoredFlag(
getSpecifierRange(ignoredFlag.getPosition(), 1)));
}
+// Determines if the specified is a C++ class or struct containing
+// a member with the specified name and kind (e.g. a CXXMethodDecl named
+// "c_str()"
+template<typename FieldKind>
Clang uses 'Field' to refer to non-static data members; MemberKind
would be clearer.
+static llvm::SmallPtrSet<FieldKind*, 1>
+CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) {
+ const RecordType *RT = Ty->getAs<RecordType>();
+ llvm::SmallPtrSet<FieldKind*, 1> Results;
+
+ if (!RT)
+ return Results;
+ const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl());
+ if (!RD)
+ return Results;
+
+ LookupResult R(S, &S.PP.getIdentifierTable().get(Name), SourceLocation(),
+ Sema::LookupMemberName);
+
+ // We just need to include all members of the right kind turned up by the
+ // filter, at this point.
+ if (S.LookupQualifiedName(R, RT->getDecl())) {
+ LookupResult::Filter filter = R.makeFilter();
You can just use LookupResult's begin/end/iterators here, rather than
building a Filter, since you're not removing elements from the
LookupResult.
+ while (filter.hasNext()) {
+ NamedDecl *decl = filter.next()->getUnderlyingDecl();
+ if (FieldKind *FK = dyn_cast<FieldKind>(decl)) {
+ Results.insert(FK);
+ }
+ }
+ filter.done();
+ }
+ return Results;
+}
+
+// Check if a (w)string was passed when a (w)char* was needed, and offer a
+// better diagnostic if so. ATR is assumed to be valid.
+// Returns true when a c_str() conversion method is found.
+bool CheckPrintfHandler::checkForCStrMembers(
+ const analyze_printf::ArgTypeResult &ATR, const Expr *E,
+ const CharSourceRange &CSR) {
+ typedef llvm::SmallPtrSet<CXXMethodDecl*, 1> MethodSet;
+
+ MethodSet Results =
+ CXXRecordMembersNamed<CXXMethodDecl>("c_str", S, E->getType());
+
+ for (MethodSet::iterator MI = Results.begin(), ME = Results.end();
+ MI != ME; ++MI) {
+ const CXXMethodDecl *Method = *MI;
+ if (Method->getNumParams() == 0 &&
+ ATR.matchesType(S.Context, Method->getResultType())) {
+
+ S.Diag(E->getLocStart(), diag::note_printf_c_str)
+ << "c_str()"
+ << FixItHint::CreateInsertion(E->getLocEnd(), ".c_str()");
+ return true;
+ }
+ }
+
+ return false;
+}
+
bool
CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier
&FS,
@@ -2396,20 +2498,30 @@
CheckPrintfHandler::HandlePrintfSpecifier(const
analyze_printf::PrintfSpecifier
if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
return false;
+ return checkFormatExpr(FS, startSpecifier, specifierLen,
+ getDataArg(argIndex));
+}
+
+bool
+CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
+ const char *StartSpecifier,
+ unsigned SpecifierLen,
+ const Expr *E) {
+ using namespace analyze_format_string;
+ using namespace analyze_printf;
// Now type check the data expression that matches the
// format specifier.
- const Expr *Ex = getDataArg(argIndex);
const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context,
ObjCContext);
- if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) {
+ if (ATR.isValid() && !ATR.matchesType(S.Context, E->getType())) {
// Look through argument promotions for our error message's reported type.
// This includes the integral and floating promotions, but excludes array
// and function pointer decay; seeing that an argument intended to be a
// string has type 'char [6]' is probably more confusing than 'char *'.
- if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Ex)) {
+ if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
if (ICE->getCastKind() == CK_IntegralCast ||
ICE->getCastKind() == CK_FloatingCast) {
- Ex = ICE->getSubExpr();
+ E = ICE->getSubExpr();
// Check if we didn't match because of an implicit cast from a 'char'
// or 'short' to an 'int'. This is done because printf is a varargs
@@ -2417,7 +2529,7 @@ CheckPrintfHandler::HandlePrintfSpecifier(const
analyze_printf::PrintfSpecifier
if (ICE->getType() == S.Context.IntTy ||
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression.
- if (ATR.matchesType(S.Context, Ex->getType()))
+ if (ATR.matchesType(S.Context, E->getType()))
return true;
}
}
@@ -2425,7 +2537,7 @@ CheckPrintfHandler::HandlePrintfSpecifier(const
analyze_printf::PrintfSpecifier
// We may be able to offer a FixItHint if it is a supported type.
PrintfSpecifier fixedFS = FS;
- bool success = fixedFS.fixType(Ex->getType(), S.getLangOpts(),
+ bool success = fixedFS.fixType(E->getType(), S.getLangOpts(),
S.Context, ObjCContext);
if (success) {
@@ -2436,24 +2548,40 @@
CheckPrintfHandler::HandlePrintfSpecifier(const
analyze_printf::PrintfSpecifier
EmitFormatDiagnostic(
S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
- << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
- << Ex->getSourceRange(),
- Ex->getLocStart(),
+ << ATR.getRepresentativeTypeName(S.Context) << E->getType()
+ << E->getSourceRange(),
+ E->getLocStart(),
/*IsStringLocation*/false,
- getSpecifierRange(startSpecifier, specifierLen),
+ getSpecifierRange(StartSpecifier, SpecifierLen),
FixItHint::CreateReplacement(
- getSpecifierRange(startSpecifier, specifierLen),
+ getSpecifierRange(StartSpecifier, SpecifierLen),
os.str()));
}
else {
+ const CharSourceRange &CSR = getSpecifierRange(StartSpecifier,
+ SpecifierLen);
+ // Since the warning for passing non-POD types to variadic functions
+ // was deferred until now, we emit a warning for non-POD
+ // arguments here.
+ if (S.levelOfTriviality(E) == Sema::PK_NON_POD) {
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_non_pod_vararg_with_format_string)
+ << E->getType()
+ << ATR.getRepresentativeTypeName(S.Context)
+ << CSR
+ << E->getSourceRange(),
+ E->getLocStart(), /*IsStringLocation*/false, CSR);
+
+ checkForCStrMembers(ATR, E, CSR);
+ }
+ else {
EmitFormatDiagnostic(
S.PDiag(diag::warn_printf_conversion_argument_type_mismatch)
- << ATR.getRepresentativeTypeName(S.Context) << Ex->getType()
- << getSpecifierRange(startSpecifier, specifierLen)
- << Ex->getSourceRange(),
- Ex->getLocStart(),
- /*IsStringLocation*/false,
- getSpecifierRange(startSpecifier, specifierLen));
+ << ATR.getRepresentativeTypeName(S.Context) << E->getType()
+ << CSR
+ << E->getSourceRange(),
+ E->getLocStart(), /*IsStringLocation*/false, CSR);
+ }
}
}
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 9f0378a..3e923e2 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -524,11 +524,38 @@ ExprResult Sema::DefaultArgumentPromotion(Expr *E) {
return Owned(E);
}
+// Determine the degree of POD-ness for an expression.
+// Incomplete types are considered POD
+Sema::PODKind Sema::levelOfTriviality(const Expr *E) {
+ if (E->getType()->isIncompleteType() || E->getType().isCXX98PODType(Context))
+ return PK_POD;
+ // C++0x [expr.call]p7:
+ // Passing a potentially-evaluated argument of class type (Clause 9)
+ // having a non-trivial copy constructor, a non-trivial move constructor,
+ // or a non-trivial destructor, with no corresponding parameter,
+ // is conditionally-supported with implementation-defined semantics.
+
+ if (getLangOpts().CPlusPlus0x && !E->getType()->isDependentType()) {
+ if (CXXRecordDecl *Record = E->getType()->getAsCXXRecordDecl()) {
+ if (Record->hasTrivialCopyConstructor() &&
+ Record->hasTrivialMoveConstructor() &&
+ Record->hasTrivialDestructor()) {
+ return PK_CXX11_POD;
+ }
+ }
+ }
+
+ if (getLangOpts().ObjCAutoRefCount && E->getType()->isObjCLifetimeType())
+ return PK_ObjC_Lifetime_POD;
+ return PK_NON_POD;
+}
+
/// DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but
/// will warn if the resulting type is not a POD type, and rejects ObjC
/// interfaces passed by value.
ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
- FunctionDecl *FDecl) {
+ FunctionDecl *FDecl,
+ bool HasFormatString) {
if (const BuiltinType *PlaceholderTy =
E->getType()->getAsPlaceholderType()) {
// Strip the unbridged-cast placeholder expression off, if applicable.
if (PlaceholderTy->getKind() == BuiltinType::ARCUnbridgedCast &&
@@ -557,37 +584,25 @@ ExprResult
Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT,
<< E->getType() << CT))
return ExprError();
+ PODKind PT = levelOfTriviality(E);
+
// Complain about passing non-POD types through varargs. However, don't
// perform this check for incomplete types, which we can get here when we're
// in an unevaluated context.
- if (!E->getType()->isIncompleteType() &&
- !E->getType().isCXX98PODType(Context)) {
- // C++0x [expr.call]p7:
- // Passing a potentially-evaluated argument of class type (Clause 9)
- // having a non-trivial copy constructor, a non-trivial move constructor,
- // or a non-trivial destructor, with no corresponding parameter,
- // is conditionally-supported with implementation-defined semantics.
- bool TrivialEnough = false;
- if (getLangOpts().CPlusPlus0x && !E->getType()->isDependentType()) {
- if (CXXRecordDecl *Record = E->getType()->getAsCXXRecordDecl()) {
- if (Record->hasTrivialCopyConstructor() &&
- Record->hasTrivialMoveConstructor() &&
- Record->hasTrivialDestructor()) {
+ switch (PT) {
+ case PK_POD: // Perfectly fine.
+ case PK_ObjC_Lifetime_POD: // Nothing to diagnose. This is okay.
+ break;
+ case PK_CXX11_POD:
DiagRuntimeBehavior(E->getLocStart(), 0,
PDiag(diag::warn_cxx98_compat_pass_non_pod_arg_to_vararg)
<< E->getType() << CT);
Indentation seems to have gone a bit weird here.
- TrivialEnough = true;
- }
- }
- }
-
- if (!TrivialEnough &&
- getLangOpts().ObjCAutoRefCount &&
- E->getType()->isObjCLifetimeType())
- TrivialEnough = true;
-
- if (TrivialEnough) {
- // Nothing to diagnose. This is okay.
+ break;
+ case PK_NON_POD:
+ if (HasFormatString) {
+ // If there is a format string argument, non-POD argument checking is
+ // handled along with format string checking.
Does this do the right thing for arguments which aren't format
arguments, for instance sprintf(my_cstr1, "%d", 1); ? Do we reach the
check in the -Wformat code if the format string is not a constant?
+ break;
} else if (DiagRuntimeBehavior(E->getLocStart(), 0,
PDiag(diag::warn_cannot_pass_non_pod_arg_to_vararg)
<< getLangOpts().CPlusPlus0x << E->getType()
@@ -3532,6 +3547,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc,
// If this is a variadic call, handle args passed through "...".
if (CallType != VariadicDoesNotApply) {
+ bool HasFormatString = hasFormatStringArgument(FDecl, Args, NumArgs);
// Assume that extern "C" functions with variadic arguments that
// return __unknown_anytype aren't *really* variadic.
@@ -3542,7 +3558,8 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc,
if (isa<ExplicitCastExpr>(Args[i]->IgnoreParens()))
arg = DefaultFunctionArrayLvalueConversion(Args[i]);
else
- arg = DefaultVariadicArgumentPromotion(Args[i], CallType, FDecl);
+ arg = DefaultVariadicArgumentPromotion(Args[i], CallType, FDecl,
+ HasFormatString);
Invalid |= arg.isInvalid();
AllArgs.push_back(arg.take());
}
@@ -3551,7 +3568,8 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc,
} else {
for (unsigned i = ArgIx; i != NumArgs; ++i) {
ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], CallType,
- FDecl);
+ FDecl,
+ HasFormatString);
Invalid |= Arg.isInvalid();
AllArgs.push_back(Arg.take());
}
diff --git a/test/SemaCXX/printf-cstr.cpp b/test/SemaCXX/printf-cstr.cpp
new file mode 100644
index 0000000..3d979a5
--- /dev/null
+++ b/test/SemaCXX/printf-cstr.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -Wall -fsyntax-only -Wformat -verify %s
+
+#include <stdarg.h>
+
+extern "C" {
+extern int printf(const char *restrict, ...);
+}
+
+class HasCStr {
+ const char *str;
+ public:
+ HasCStr(const char *s): str(s) { }
+ const char *c_str() {return str;}
+};
+
+class HasNoCStr {
+ const char *str;
+ public:
+ HasNoCStr(const char *s): str(s) { }
+ const char *not_c_str() {return str;}
+};
+
+void f() {
+ char str[] = "test";
+ HasCStr hcs(str);
+ HasNoCStr hncs(str);
+ printf("%d: %s\n", 10, hcs.c_str());
+ printf("%d: %s\n", 10, hcs); // expected-error{{cannot pass non-POD
object of type 'HasCStr' to variadic function; expected type from
format string was 'char *'}} expected-note{{did you mean to call the
c_str() method?}}
+ printf("%d: %s\n", 10, hncs); // expected-error{{cannot pass
non-POD object of type 'HasNoCStr' to variadic function; expected type
from format string was 'char *'}}
+}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120613/d22c69df/attachment.html>
More information about the cfe-commits
mailing list