[cfe-commits] r83764 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/CMakeLists.txt lib/Sema/Sema.h lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExceptionSpec.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaType.cpp test/SemaCXX/exception-spec.cpp

[Douglas Gregor]

On Oct 11, 2009, at 2:03 AM, Sebastian Redl wrote:
> Author: cornedbee
> Date: Sun Oct 11 04:03:14 2009
> New Revision: 83764
>
> URL: http://llvm.org/viewvc/llvm-project?rev=83764&view=rev
> Log:
> Test exception spec compatibility on return type and parameters.
>
> Along the way, use RequireCompleteType when testing exception spec  
> types.
> Separate all the ugly spec stuff into its own file.

Cool. Some comments below.

> +def err_return_type_specs_differ : Error<
> +  "exception specifications of return types differ">;
> +def err_arg_type_specs_differ : Error<
> +  "exception specifications of argument types differ">;

Knowing how they differ would be helpful; again, not critical.

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaExceptionSpec.cpp (added)
> +++ cfe/trunk/lib/Sema/SemaExceptionSpec.cpp Sun Oct 11 04:03:14 2009
> @@ -0,0 +1,309 @@
> +//===--- SemaExceptionSpec.cpp - C++ Exception Specifications ---*-  
> C++ -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open  
> Source
> +// License. See LICENSE.TXT for details.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//
> +// This file provides Sema routines for C++ exception specification  
> testing.
> +//
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +
> +#include "Sema.h"
> +#include "clang/Basic/Diagnostic.h"
> +#include "clang/AST/CXXInheritance.h"
> +#include "clang/AST/Expr.h"
> +#include "clang/AST/ExprCXX.h"
> +#include "llvm/ADT/SmallPtrSet.h"
> +
> +namespace clang {
> +
> +static const FunctionProtoType *GetUnderlyingFunction(QualType T)
> +{
> +  if (const PointerType *PtrTy = T->getAs<PointerType>())
> +    T = PtrTy->getPointeeType();
> +  else if (const ReferenceType *RefTy = T->getAs<ReferenceType>())
> +    T = RefTy->getPointeeType();
> +  return T->getAs<FunctionProtoType>();
> +}
> +
> +/// CheckSpecifiedExceptionType - Check if the given type is valid  
> in an
> +/// exception specification. Incomplete types, or pointers to  
> incomplete types
> +/// other than void are not allowed.
> +bool Sema::CheckSpecifiedExceptionType(QualType T, const  
> SourceRange &Range) {
> +  // FIXME: This may not correctly work with the fix for core issue  
> 437,
> +  // where a class's own type is considered complete within its  
> body. But
> +  // perhaps RequireCompleteType itself should contain this logic?

My suggestion for this would be to have the parser delay semantic  
analysis of the exception specifications of member functions in the  
same way that we delay parsing of default arguments of the parameters  
of member functions. For example, these checks on the exception  
specialization for a member function declaration could be performed by  
ActOnFinishDelayedCXXMethodDeclaration.

> +  // C++ 15.4p2: A type denoted in an exception-specification shall  
> not denote
> +  //   an incomplete type.
> +  // FIXME: This isn't right. This will supress diagnostics from  
> template
> +  // instantiation and then simply emit the invalid type diagnostic.
> +  if (RequireCompleteType(Range.getBegin(), T, 0))
> +    return Diag(Range.getBegin(),  
> diag::err_incomplete_in_exception_spec)
> +      << Range << T << /*direct*/0;

You should be able to use partial diagnostics to get the diagnostic  
you want within RequireCompleteType.

> +  // C++ 15.4p2: A type denoted in an exception-specification shall  
> not denote
> +  //   an incomplete type a pointer or reference to an incomplete  
> type, other
> +  //   than (cv) void*.
> +  int kind;
> +  if (const PointerType* IT = T->getAs<PointerType>()) {
> +    T = IT->getPointeeType();
> +    kind = 1;
> +  } else if (const ReferenceType* IT = T->getAs<ReferenceType>()) {
> +    T = IT->getPointeeType();
> +    kind = 2;
> +  } else
> +    return false;
> +
> +  if (!T->isVoidType() && RequireCompleteType(Range.getBegin(), T,  
> 0))
> +    return Diag(Range.getBegin(),  
> diag::err_incomplete_in_exception_spec)
> +      << Range << T << /*indirect*/kind;

Same thing here: partial diagnostics should be helpful.
>
> +  llvm::SmallPtrSet<const Type*, 8> Types;

Should this be a

   llvm::SmallPtrSet<CanQualType, 8> Types;

?

> +/// CheckExceptionSpecSubset - Check whether the second function  
> type's
> +/// exception specification is a subset (or equivalent) of the  
> first function
> +/// type. This is used by override and pointer assignment checks.
> +bool Sema::CheckExceptionSpecSubset(unsigned DiagID, unsigned NoteID,
> +    const FunctionProtoType *Superset, SourceLocation SuperLoc,
> +    const FunctionProtoType *Subset, SourceLocation SubLoc) {
> +  // FIXME: As usual, we could be more specific in our error  
> messages, but
> +  // that better waits until we've got types with source locations.
> +
> +  if (!SubLoc.isValid())
> +    SubLoc = SuperLoc;
> +
> +  // If superset contains everything, we're done.
> +  if (!Superset->hasExceptionSpec() || Superset->hasAnyExceptionSpec 
> ())
> +    return CheckParamExceptionSpec(NoteID, Superset, SuperLoc,  
> Subset, SubLoc);
> +
> +  // It does not. If the subset contains everything, we've failed.
> +  if (!Subset->hasExceptionSpec() || Subset->hasAnyExceptionSpec()) {
> +    Diag(SubLoc, DiagID);
> +    if (NoteID != 0)
> +      Diag(SuperLoc, NoteID);
> +    return true;
> +  }
> +
> +  // Neither contains everything. Do a proper comparison.
> +  for (FunctionProtoType::exception_iterator SubI = Subset- 
> >exception_begin(),
> +       SubE = Subset->exception_end(); SubI != SubE; ++SubI) {
> +    // Take one type from the subset.
> +    QualType CanonicalSubT = Context.getCanonicalType(*SubI);
> +    bool SubIsPointer = false;
> +    if (const ReferenceType *RefTy = CanonicalSubT- 
> >getAs<ReferenceType>())
> +      CanonicalSubT = RefTy->getPointeeType();
> +    if (const PointerType *PtrTy = CanonicalSubT->getAs<PointerType> 
> ()) {
> +      CanonicalSubT = PtrTy->getPointeeType();
> +      SubIsPointer = true;
> +    }

Are there pointer-to-member cases we need to check here?

> +    bool SubIsClass = CanonicalSubT->isRecordType();
> +    CanonicalSubT = CanonicalSubT.getUnqualifiedType();
> +
> +    CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/ 
> true,
> +                       /*DetectVirtual=*/false);
> +
> +    bool Contained = false;
> +    // Make sure it's in the superset.
> +    for (FunctionProtoType::exception_iterator SuperI =
> +           Superset->exception_begin(), SuperE = Superset- 
> >exception_end();
> +         SuperI != SuperE; ++SuperI) {
> +      QualType CanonicalSuperT = Context.getCanonicalType(*SuperI);
> +      // SubT must be SuperT or derived from it, or pointer or  
> reference to
> +      // such types.
> +      if (const ReferenceType *RefTy = CanonicalSuperT- 
> >getAs<ReferenceType>())
> +        CanonicalSuperT = RefTy->getPointeeType();
> +      if (SubIsPointer) {
> +        if (const PointerType *PtrTy = CanonicalSuperT- 
> >getAs<PointerType>())
> +          CanonicalSuperT = PtrTy->getPointeeType();
> +        else {
> +          continue;
> +        }
> +      }

(same pointer-to-member question here)

Very nice!

	- Doug