From 67ebdeb467bb0a2e24caf4d58899d861ba655201 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 17 Feb 2016 00:04:04 +0000 Subject: [PATCH] Improve diagnostics for ill-formed literal operator declarations. Patch by Erik Pilkington! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@261034 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 13 +- lib/Sema/SemaDeclCXX.cpp | 230 ++++++++++++-------- test/CXX/over/over.oper/over.literal/p5.cpp | 12 +- test/CXX/over/over.oper/over.literal/p8.cpp | 2 +- test/SemaCXX/literal-operators.cpp | 7 +- 5 files changed, 165 insertions(+), 99 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index ca0d3834cd..4b00445414 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6886,9 +6886,16 @@ def err_literal_operator_id_outside_namespace : Error< "non-namespace scope '%0' cannot have a literal operator member">; def err_literal_operator_default_argument : Error< "literal operator cannot have a default argument">; -// FIXME: This diagnostic sucks -def err_literal_operator_params : Error< - "parameter declaration for literal operator %0 is not valid">; +def err_literal_operator_bad_param_count : Error< + "non-template literal operator must have one or two parameters">; +def err_literal_operator_invalid_param : Error< + "parameter of literal operator must have type 'unsigned long long', 'long double', 'char', 'wchar_t', 'char16_t', 'char32_t', or 'const char *'">; +def err_literal_operator_param : Error< + "invalid literal operator parameter type %0, did you mean %1?">; +def err_literal_operator_template_with_params : Error< + "literal operator template cannot have any parameters">; +def err_literal_operator_template : Error< + "template parameter list for literal operator must be either 'char...' or 'typename T, T...'">; def err_literal_operator_extern_c : Error< "literal operator must have C++ linkage">; def ext_string_literal_operator_template : ExtWarn< diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index b7e03e3eb6..c160f444ca 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -11765,6 +11765,49 @@ bool Sema::CheckOverloadedOperatorDeclaration(FunctionDecl *FnDecl) { return false; } +static bool +checkLiteralOperatorTemplateParameterList(Sema &SemaRef, + FunctionTemplateDecl *TpDecl) { + TemplateParameterList *TemplateParams = TpDecl->getTemplateParameters(); + + // Must have one or two template parameters. + if (TemplateParams->size() == 1) { + NonTypeTemplateParmDecl *PmDecl = + dyn_cast(TemplateParams->getParam(0)); + + // The template parameter must be a char parameter pack. + if (PmDecl && PmDecl->isTemplateParameterPack() && + SemaRef.Context.hasSameType(PmDecl->getType(), SemaRef.Context.CharTy)) + return false; + + } else if (TemplateParams->size() == 2) { + TemplateTypeParmDecl *PmType = + dyn_cast(TemplateParams->getParam(0)); + NonTypeTemplateParmDecl *PmArgs = + dyn_cast(TemplateParams->getParam(1)); + + // The second template parameter must be a parameter pack with the + // first template parameter as its type. + if (PmType && PmArgs && !PmType->isTemplateParameterPack() && + PmArgs->isTemplateParameterPack()) { + const TemplateTypeParmType *TArgs = + PmArgs->getType()->getAs(); + if (TArgs && TArgs->getDepth() == PmType->getDepth() && + TArgs->getIndex() == PmType->getIndex()) { + if (SemaRef.ActiveTemplateInstantiations.empty()) + SemaRef.Diag(TpDecl->getLocation(), + diag::ext_string_literal_operator_template); + return false; + } + } + } + + SemaRef.Diag(TpDecl->getTemplateParameters()->getSourceRange().getBegin(), + diag::err_literal_operator_template) + << TpDecl->getTemplateParameters()->getSourceRange(); + return true; +} + /// CheckLiteralOperatorDeclaration - Check whether the declaration /// of this literal operator function is well-formed. If so, returns /// false; otherwise, emits appropriate diagnostics and returns true. @@ -11780,10 +11823,9 @@ bool Sema::CheckLiteralOperatorDeclaration(FunctionDecl *FnDecl) { return true; } - bool Valid = false; - // This might be the definition of a literal operator template. FunctionTemplateDecl *TpDecl = FnDecl->getDescribedFunctionTemplate(); + // This might be a specialization of a literal operator template. if (!TpDecl) TpDecl = FnDecl->getPrimaryTemplate(); @@ -11792,101 +11834,117 @@ bool Sema::CheckLiteralOperatorDeclaration(FunctionDecl *FnDecl) { // template type operator "" name() are the only valid // template signatures, and the only valid signatures with no parameters. if (TpDecl) { - if (FnDecl->param_size() == 0) { - // Must have one or two template parameters - TemplateParameterList *Params = TpDecl->getTemplateParameters(); - if (Params->size() == 1) { - NonTypeTemplateParmDecl *PmDecl = - dyn_cast(Params->getParam(0)); - - // The template parameter must be a char parameter pack. - if (PmDecl && PmDecl->isTemplateParameterPack() && - Context.hasSameType(PmDecl->getType(), Context.CharTy)) - Valid = true; - } else if (Params->size() == 2) { - TemplateTypeParmDecl *PmType = - dyn_cast(Params->getParam(0)); - NonTypeTemplateParmDecl *PmArgs = - dyn_cast(Params->getParam(1)); - - // The second template parameter must be a parameter pack with the - // first template parameter as its type. - if (PmType && PmArgs && - !PmType->isTemplateParameterPack() && - PmArgs->isTemplateParameterPack()) { - const TemplateTypeParmType *TArgs = - PmArgs->getType()->getAs(); - if (TArgs && TArgs->getDepth() == PmType->getDepth() && - TArgs->getIndex() == PmType->getIndex()) { - Valid = true; - if (ActiveTemplateInstantiations.empty()) - Diag(FnDecl->getLocation(), - diag::ext_string_literal_operator_template); - } - } + if (FnDecl->param_size() != 0) { + Diag(FnDecl->getLocation(), + diag::err_literal_operator_template_with_params); + return true; + } + + if (checkLiteralOperatorTemplateParameterList(*this, TpDecl)) + return true; + + } else if (FnDecl->param_size() == 1) { + const ParmVarDecl *Param = FnDecl->getParamDecl(0); + + QualType ParamType = Param->getType().getUnqualifiedType(); + + // Only unsigned long long int, long double, any character type, and const + // char * are allowed as the only parameters. + if (ParamType->isSpecificBuiltinType(BuiltinType::ULongLong) || + ParamType->isSpecificBuiltinType(BuiltinType::LongDouble) || + Context.hasSameType(ParamType, Context.CharTy) || + Context.hasSameType(ParamType, Context.WideCharTy) || + Context.hasSameType(ParamType, Context.Char16Ty) || + Context.hasSameType(ParamType, Context.Char32Ty)) { + } else if (const PointerType *Ptr = ParamType->getAs()) { + QualType InnerType = Ptr->getPointeeType(); + + // Pointer parameter must be a const char *. + if (!(Context.hasSameType(InnerType.getUnqualifiedType(), + Context.CharTy) && + InnerType.isConstQualified() && !InnerType.isVolatileQualified())) { + Diag(Param->getSourceRange().getBegin(), + diag::err_literal_operator_param) + << ParamType << "'const char *'" << Param->getSourceRange(); + return true; } + + } else if (ParamType->isRealFloatingType()) { + Diag(Param->getSourceRange().getBegin(), diag::err_literal_operator_param) + << ParamType << Context.LongDoubleTy << Param->getSourceRange(); + return true; + + } else if (ParamType->isIntegerType()) { + Diag(Param->getSourceRange().getBegin(), diag::err_literal_operator_param) + << ParamType << Context.UnsignedLongLongTy << Param->getSourceRange(); + return true; + + } else { + Diag(Param->getSourceRange().getBegin(), + diag::err_literal_operator_invalid_param) + << ParamType << Param->getSourceRange(); + return true; } - } else if (FnDecl->param_size()) { - // Check the first parameter + + } else if (FnDecl->param_size() == 2) { FunctionDecl::param_iterator Param = FnDecl->param_begin(); - QualType T = (*Param)->getType().getUnqualifiedType(); - - // unsigned long long int, long double, and any character type are allowed - // as the only parameters. - if (Context.hasSameType(T, Context.UnsignedLongLongTy) || - Context.hasSameType(T, Context.LongDoubleTy) || - Context.hasSameType(T, Context.CharTy) || - Context.hasSameType(T, Context.WideCharTy) || - Context.hasSameType(T, Context.Char16Ty) || - Context.hasSameType(T, Context.Char32Ty)) { - if (++Param == FnDecl->param_end()) - Valid = true; - goto FinishedParams; - } - - // Otherwise it must be a pointer to const; let's strip those qualifiers. - const PointerType *PT = T->getAs(); - if (!PT) - goto FinishedParams; - T = PT->getPointeeType(); - if (!T.isConstQualified() || T.isVolatileQualified()) - goto FinishedParams; - T = T.getUnqualifiedType(); - - // Move on to the second parameter; - ++Param; + // First, verify that the first parameter is correct. + + QualType FirstParamType = (*Param)->getType().getUnqualifiedType(); + + // Two parameter function must have a pointer to const as a + // first parameter; let's strip those qualifiers. + const PointerType *PT = FirstParamType->getAs(); - // If there is no second parameter, the first must be a const char * - if (Param == FnDecl->param_end()) { - if (Context.hasSameType(T, Context.CharTy)) - Valid = true; - goto FinishedParams; + if (!PT) { + Diag((*Param)->getSourceRange().getBegin(), + diag::err_literal_operator_param) + << FirstParamType << "'const char *'" << (*Param)->getSourceRange(); + return true; } - // const char *, const wchar_t*, const char16_t*, and const char32_t* + QualType PointeeType = PT->getPointeeType(); + // First parameter must be const + if (!PointeeType.isConstQualified() || PointeeType.isVolatileQualified()) { + Diag((*Param)->getSourceRange().getBegin(), + diag::err_literal_operator_param) + << FirstParamType << "'const char *'" << (*Param)->getSourceRange(); + return true; + } + + QualType InnerType = PointeeType.getUnqualifiedType(); + // Only const char *, const wchar_t*, const char16_t*, and const char32_t* // are allowed as the first parameter to a two-parameter function - if (!(Context.hasSameType(T, Context.CharTy) || - Context.hasSameType(T, Context.WideCharTy) || - Context.hasSameType(T, Context.Char16Ty) || - Context.hasSameType(T, Context.Char32Ty))) - goto FinishedParams; - - // The second and final parameter must be an std::size_t - T = (*Param)->getType().getUnqualifiedType(); - if (Context.hasSameType(T, Context.getSizeType()) && - ++Param == FnDecl->param_end()) - Valid = true; - } - - // FIXME: This diagnostic is absolutely terrible. -FinishedParams: - if (!Valid) { - Diag(FnDecl->getLocation(), diag::err_literal_operator_params) - << FnDecl->getDeclName(); + if (!(Context.hasSameType(InnerType, Context.CharTy) || + Context.hasSameType(InnerType, Context.WideCharTy) || + Context.hasSameType(InnerType, Context.Char16Ty) || + Context.hasSameType(InnerType, Context.Char32Ty))) { + Diag((*Param)->getSourceRange().getBegin(), + diag::err_literal_operator_param) + << FirstParamType << "'const char *'" << (*Param)->getSourceRange(); + return true; + } + + // Move on to the second and final parameter. + ++Param; + + // The second parameter must be a std::size_t. + QualType SecondParamType = (*Param)->getType().getUnqualifiedType(); + if (!Context.hasSameType(SecondParamType, Context.getSizeType())) { + Diag((*Param)->getSourceRange().getBegin(), + diag::err_literal_operator_param) + << SecondParamType << Context.getSizeType() + << (*Param)->getSourceRange(); + return true; + } + } else { + Diag(FnDecl->getLocation(), diag::err_literal_operator_bad_param_count); return true; } + // Parameters are good. + // A parameter-declaration-clause containing a default argument is not // equivalent to any of the permitted forms. for (auto Param : FnDecl->params()) { diff --git a/test/CXX/over/over.oper/over.literal/p5.cpp b/test/CXX/over/over.oper/over.literal/p5.cpp index 66f3f97eaa..bfad5f00cf 100644 --- a/test/CXX/over/over.oper/over.literal/p5.cpp +++ b/test/CXX/over/over.oper/over.literal/p5.cpp @@ -12,11 +12,11 @@ template struct U { friend U operator "" _a(const T *, size_t); // expected-error {{parameter}} }; template struct V { - friend void operator "" _b(); // expected-error {{parameter}} + friend void operator "" _b(); // expected-error {{parameters}} }; -template void operator "" _b(); // expected-error {{parameter}} -template void operator "" _b(int N = 0); // expected-error {{parameter}} -template void operator "" _b(); // expected-error {{parameter}} -template T operator "" _b(const char *); // expected-error {{parameter}} -template int operator "" _b(const T *, size_t); // expected-error {{parameter}} +template void operator "" _b(); // expected-error {{template}} +template void operator "" _b(int N = 0); // expected-error {{template}} +template void operator "" _b(); // expected-error {{template}} +template T operator "" _b(const char *); // expected-error {{template}} +template int operator "" _b(const T *, size_t); // expected-error {{template}} diff --git a/test/CXX/over/over.oper/over.literal/p8.cpp b/test/CXX/over/over.oper/over.literal/p8.cpp index 70a184372c..6644bae7e6 100644 --- a/test/CXX/over/over.oper/over.literal/p8.cpp +++ b/test/CXX/over/over.oper/over.literal/p8.cpp @@ -12,6 +12,6 @@ float operator ""E(const char *); // expected-error {{invalid suffix on literal} float operator " " B(const char *); // expected-error {{must be '""'}} expected-warning {{reserved}} string operator "" 5X(const char *, std::size_t); // expected-error {{expected identifier}} double operator "" _miles(double); // expected-error {{parameter}} -template int operator "" j(const char*); // expected-error {{parameter}} +template int operator "" j(const char*); // expected-error {{template}} float operator ""_E(const char *); diff --git a/test/SemaCXX/literal-operators.cpp b/test/SemaCXX/literal-operators.cpp index ba571788b9..304aa7cab7 100644 --- a/test/SemaCXX/literal-operators.cpp +++ b/test/SemaCXX/literal-operators.cpp @@ -35,13 +35,14 @@ typedef const char c; void operator "" _good (c*); // Check extra cv-qualifiers -void operator "" _cv_good (volatile const char *, const size_t); // expected-error {{parameter declaration for literal operator 'operator""_cv_good' is not valid}} +void operator "" _cv_good (volatile const char *, const size_t); // expected-error {{invalid literal operator parameter type 'const volatile char *', did you mean 'const char *'?}} // Template declaration template void operator "" _good (); -// FIXME: Test some invalid decls that might crop up. -template void operator "" _invalid(); // expected-error {{parameter declaration for literal operator 'operator""_invalid' is not valid}} +template void operator "" _invalid(); // expected-error {{template parameter list for literal operator must be either 'char...' or 'typename T, T...'}} +template void operator "" _invalid(); // expected-error {{template parameter list for literal operator must be either 'char...' or 'typename T, T...'}} +template void operator "" _invalid(); // expected-error {{template parameter list for literal operator must be either 'char...' or 'typename T, T...'}} _Complex float operator""if(long double); // expected-warning {{reserved}} _Complex float test_if_1() { return 2.0f + 1.5if; }; -- 2.40.0