This is an archive of the discontinued LLVM Phabricator instance.

[SemaTemplate] Mark a function type as dependent when its parameter list contains pack expansion
Needs ReviewPublic

Authored by cpplearner on Jul 21 2019, 6:32 AM.

Details

Summary

Given template<class> using Int = int;, the type void(Int<Ts>...) should be treated as a dependent type, even though Int<Ts> is not dependent.

This fixes https://bugs.llvm.org/show_bug.cgi?id=42654

Diff Detail

Repository
rC Clang

Event Timeline

cpplearner created this revision.Jul 21 2019, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 21 2019, 6:32 AM
efriedma added a subscriber: efriedma.

Is this the only place where a compound type can contain a PackExpansionType?

The code could probably use a brief comment explaining why we need to check this explicitly, even though it isn't listed in [temp.dep.type].

Is this the only place where a compound type can contain a PackExpansionType?

Yes AFAIK. No other place can contain a list or a pack expansion. The closest thing is dynamic exception specification (which isn't part of the type), and it seems to be handled properly.

The code could probably use a brief comment explaining why we need to check this explicitly, even though it isn't listed in [temp.dep.type].

Done.

Is this the only place where a compound type can contain a PackExpansionType?

Yes AFAIK. No other place can contain a list or a pack expansion. The closest thing is dynamic exception specification (which isn't part of the type), and it seems to be handled properly.

The noexcept specifier is part of the type these days, is that also handled properly?

The noexcept specifier is part of the type these days, is that also handled properly?

I believe that it's properly handled in this section of FunctionProtoType::FunctionProtoType:

// If this is a canonical type, and its exception specification is dependent,
// then it's a dependent type. This only happens in C++17 onwards.
if (isCanonicalUnqualified()) {
  if (getExceptionSpecType() == EST_Dynamic ||
      getExceptionSpecType() == EST_DependentNoexcept) {
    assert(hasDependentExceptionSpec() && "type should not be canonical");
    setDependent();
  }
} else if (getCanonicalTypeInternal()->isDependentType()) {
  // Ask our canonical type whether our exception specification was dependent.
  setDependent();
}

I think this is reasonable, but I'd like @rsmith to weigh in -- this looks like it could be a Core Issue.

this looks like it could be a Core Issue

I think the issue is clang-specific. clang splits the standard notion of a dependent type into two separate bits, for the sake of diagnostics: isDependentType(), and isInstantiationDependentType(). isInstantiationDependentType() reflects the actual standard definition of a dependent type; isDependentType() is a type that can actually vary across instantiations.

this looks like it could be a Core Issue

I think the issue is clang-specific. clang splits the standard notion of a dependent type into two separate bits, for the sake of diagnostics: isDependentType(), and isInstantiationDependentType(). isInstantiationDependentType() reflects the actual standard definition of a dependent type; isDependentType() is a type that can actually vary across instantiations.

According to comment in include/clang/AST/Type.h (https://github.com/llvm/llvm-project/blob/llvmorg-8.0.1/clang/include/clang/AST/Type.h#L1426), Dependent reflects the standard definition, while InstantiationDependent reflects "whether this type somehow involves a template parameter, even if the resolution of the type does not depend on a template parameter" (e.g. decltype(sizeof(T))).

Therefore, I agree with Aaron that this could be a Core issue.