r217995 - Instantiate exception specifications when instantiating function types (other

[Alexander Kornienko]

On Thu, Sep 18, 2014 at 4:32 PM, Alexander Kornienko <alexfh at google.com>
wrote:

> On Thu, Sep 18, 2014 at 1:57 AM, Richard Smith <richard-llvm at metafoo.co.uk
> > wrote:
>
>> Author: rsmith
>> Date: Wed Sep 17 18:57:05 2014
>> New Revision: 217995
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=217995&view=rev
>> Log:
>> Instantiate exception specifications when instantiating function types
>> (other
>> than the type of a function declaration). We previously didn't instantiate
>> these at all! This also covers the pathological case where the only
>> mention of
>> a parameter pack is within the exception specification; this gives us a
>> second
>> way (other than alias templates) to reach the horrible state where a type
>> contains an unexpanded pack, but its canonical type does not.
>>
>> Modified:
>>     cfe/trunk/include/clang/AST/DataRecursiveASTVisitor.h
>>     cfe/trunk/include/clang/AST/Expr.h
>>     cfe/trunk/include/clang/AST/RecursiveASTVisitor.h
>>     cfe/trunk/include/clang/AST/Type.h
>>     cfe/trunk/include/clang/Sema/Sema.h
>>     cfe/trunk/lib/AST/Type.cpp
>>     cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>>     cfe/trunk/lib/Sema/SemaExceptionSpec.cpp
>>     cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
>>     cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
>>     cfe/trunk/lib/Sema/SemaType.cpp
>>     cfe/trunk/lib/Sema/TreeTransform.h
>>     cfe/trunk/test/CXX/except/except.spec/p1.cpp
>>     cfe/trunk/test/SemaTemplate/instantiate-exception-spec-cxx11.cpp
>>     cfe/trunk/test/SemaTemplate/instantiate-exception-spec.cpp
>>
>> ...
>>
>> Modified: cfe/trunk/lib/AST/Type.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=217995&r1=217994&r2=217995&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/AST/Type.cpp (original)
>> +++ cfe/trunk/lib/AST/Type.cpp Wed Sep 17 18:57:05 2014
>> @@ -1623,9 +1623,9 @@ FunctionProtoType::FunctionProtoType(Qua
>>      QualType *exnSlot = argSlot + NumParams;
>>      unsigned I = 0;
>>      for (QualType ExceptionType : epi.ExceptionSpec.Exceptions) {
>> -      if (ExceptionType->isDependentType())
>> -        setDependent();
>> -      else if (ExceptionType->isInstantiationDependentType())
>> +      // Note that a dependent exception specification does *not* make
>> +      // a type dependent; it's not even part of the C++ type system.
>> +      if (ExceptionType->isInstantiationDependentType())
>>          setInstantiationDependent();
>>
>>        if (ExceptionType->containsUnexpandedParameterPack())
>> @@ -1639,11 +1639,12 @@ FunctionProtoType::FunctionProtoType(Qua
>>      *noexSlot = epi.ExceptionSpec.NoexceptExpr;
>>
>>      if (epi.ExceptionSpec.NoexceptExpr) {
>> -      if (epi.ExceptionSpec.NoexceptExpr->isValueDependent()
>> -          || epi.ExceptionSpec.NoexceptExpr->isTypeDependent())
>> -        setDependent();
>> -      else if
>> (epi.ExceptionSpec.NoexceptExpr->isInstantiationDependent())
>> +      if (epi.ExceptionSpec.NoexceptExpr->isValueDependent() ||
>> +          epi.ExceptionSpec.NoexceptExpr->isInstantiationDependent())
>>          setInstantiationDependent();
>> +
>> +      if
>> (epi.ExceptionSpec.NoexceptExpr->containsUnexpandedParameterPack())
>> +        setContainsUnexpandedParameterPack();
>>      }
>>    } else if (getExceptionSpecType() == EST_Uninstantiated) {
>>      // Store the function decl from which we will resolve our
>> @@ -1669,6 +1670,19 @@ FunctionProtoType::FunctionProtoType(Qua
>>    }
>>  }
>>
>> +bool FunctionProtoType::hasDependentExceptionSpec() const {
>> +  if (Expr *NE = getNoexceptExpr())
>> +    return NE->isValueDependent();
>> +  for (unsigned I = 0, N = getNumExceptions(); I != N; ++I)
>>
>
> This crashes on test/CXX/except/except.spec/template.cpp in certain build
> configurations. I suspect, the reason is the complicated relation between
> getNumExceptions/getExceptionType and exception_begin/exception_end:
>
>   exception_iterator exception_end() const {
>     if (getExceptionSpecType() != EST_Dynamic)
>       return exception_begin();
>     return exception_begin() + NumExceptions;
>   }
> ...
>   unsigned getNumExceptions() const { return NumExceptions; }
>   QualType getExceptionType(unsigned i) const {
>     assert(i < NumExceptions && "Invalid exception number!");
>     return exception_begin()[i];
>   }
>
> I wonder why the two ways to access exceptions are different. It looks
> like a bug.
>
> Actually, I don't think we need getNumExceptions/getExceptionType, since
> there is ArrayRef<QualType> exceptions();.
>

I've changed the code to use exceptions() instead of
getNumExceptions/getExceptionType in r218053. All tests pass and the crash
in my setup disappeared, so I assume that this is the right thing to do ;)


>
>
>> +    // A pack expansion with a non-dependent pattern is still dependent,
>> +    // because we don't know whether the pattern is in the exception spec
>> +    // or not (that depends on whether the pack has 0 expansions).
>> +    if (getExceptionType(I)->isDependentType() ||
>> +        getExceptionType(I)->getAs<PackExpansionType>())
>> +      return true;
>> +  return false;
>> +}
>> +
>>  FunctionProtoType::NoexceptResult
>>  FunctionProtoType::getNoexceptSpec(const ASTContext &ctx) const {
>>    ExceptionSpecificationType est = getExceptionSpecType();
>> ...
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140918/1048f18a/attachment.html>