r295016 - Canonicalize implicit deduction guide parameter types when forming a deduction

[Eric Fiselier via cfe-commits]

Hi Richard,

The following test triggers an assertion, even after this patch:

#include <memory>
#include <string>

template <class T>
struct TestAlloc : std::allocator<T> {};

template <class Char, class Traits = std::char_traits<Char>,
          class Allocator = std::allocator<Char>>
struct String {
  using size_type = typename Allocator::size_type;

  String() = default;
  String(const String&, size_type, size_type, const Allocator& =
Allocator()) {}
  String(String const&, size_type, const Allocator& = Allocator()) {}
};

int main() {
  using BStr = String<char>;
  BStr bs;
  String s(bs, 1ul, 1ul, std::allocator<char>{});
  static_assert(std::is_same_v<decltype(s), BStr>);
}


On Mon, Feb 13, 2017 at 6:50 PM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: rsmith
> Date: Mon Feb 13 19:49:59 2017
> New Revision: 295016
>
> URL: http://llvm.org/viewvc/llvm-project?rev=295016&view=rev
> Log:
> Canonicalize implicit deduction guide parameter types when forming a
> deduction
> guide from a constructor.
>
> The purpose of this change is to avoid triggering instantiation of the
> class
> when substituting back into the deduction guide if it uses a typedef
> member.
> We will still instantiate the class if the constructor (explicitly or
> implicitly, directly or indirectly) uses the current instantiation in a way
> that we can't canonicalize out, but that seems unavoidable.
>
> Modified:
>     cfe/trunk/lib/Sema/SemaTemplate.cpp
>     cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaTemplate.cpp?rev=295016&r1=295015&r2=295016&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaTemplate.cpp Mon Feb 13 19:49:59 2017
> @@ -1585,12 +1585,7 @@ private:
>
>      //    -- The types of the function parameters are those of the
> constructor.
>      for (auto *OldParam : TL.getParams()) {
> -      // If we're transforming a non-template constructor, just reuse its
> -      // parameters as the parameters of the deduction guide. Otherwise,
> we
> -      // need to transform their references to constructor template
> parameters.
> -      ParmVarDecl *NewParam = Args.getNumLevels()
> -                                  ? transformFunctionTypeParam(OldParam,
> Args)
> -                                  : OldParam;
> +      ParmVarDecl *NewParam = transformFunctionTypeParam(OldParam, Args);
>        if (!NewParam)
>          return QualType();
>        ParamTypes.push_back(NewParam->getType());
> @@ -1636,16 +1631,31 @@ private:
>    transformFunctionTypeParam(ParmVarDecl *OldParam,
>                               MultiLevelTemplateArgumentList &Args) {
>      TypeSourceInfo *OldDI = OldParam->getTypeSourceInfo();
> -    TypeSourceInfo *NewDI = SemaRef.SubstType(
> -        OldDI, Args, OldParam->getLocation(), OldParam->getDeclName());
> +    TypeSourceInfo *NewDI =
> +        Args.getNumLevels()
> +            ? SemaRef.SubstType(OldDI, Args, OldParam->getLocation(),
> +                                OldParam->getDeclName())
> +            : OldDI;
>      if (!NewDI)
>        return nullptr;
>
> +    // Canonicalize the type. This (for instance) replaces references to
> +    // typedef members of the current instantiations with the definitions
> of
> +    // those typedefs, avoiding triggering instantiation of the deduced
> type
> +    // during deduction.
> +    // FIXME: It would be preferable to retain type sugar and source
> +    // information here (and handle this in substitution instead).
> +    NewDI = SemaRef.Context.getTrivialTypeSourceInfo(
> +        SemaRef.Context.getCanonicalType(NewDI->getType()),
> +        OldParam->getLocation());
> +
>      // Resolving a wording defect, we also inherit default arguments from
> the
>      // constructor.
>      ExprResult NewDefArg;
>      if (OldParam->hasDefaultArg()) {
> -      NewDefArg = SemaRef.SubstExpr(OldParam->getDefaultArg(), Args);
> +      NewDefArg = Args.getNumLevels()
> +                      ? SemaRef.SubstExpr(OldParam->getDefaultArg(),
> Args)
> +                      : OldParam->getDefaultArg();
>        if (NewDefArg.isInvalid())
>          return nullptr;
>      }
>
> Modified: cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-
> deduction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/cxx1z-class-template-argument-deduction.cpp?rev=
> 295016&r1=295015&r2=295016&view=diff
> ============================================================
> ==================
> --- cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
> (original)
> +++ cfe/trunk/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
> Mon Feb 13 19:49:59 2017
> @@ -136,4 +136,17 @@ namespace look_into_current_instantiatio
>      B(typename X::type); // expected-note {{couldn't infer template
> argument 'T'}}
>    };
>    B b = 0; // expected-error {{no viable}}
> +
> +  // We should have a substitution failure in the immediate context of
> +  // deduction when using the C(T, U) constructor (probably; core wording
> +  // unclear).
> +  template<typename T> struct C {
> +    using U = typename T::type;
> +    C(T, U);
> +  };
> +
> +  struct R { R(int); typedef R type; };
> +  C(...) -> C<R>;
> +
> +  C c = {1, 2};
>  }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170213/54ca2dd6/attachment.html>