PCH serialization issues (PR 34728)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

PCH serialization issues (PR 34728)

David Chisnall via cfe-dev
I'm running into trouble when serializing a template function
declaration to PCH and subsequently reading it back:
https://bugs.llvm.org/show_bug.cgi?id=34728

Small example:

  header.h
  --
  template <int i=42, class T> int f(T const&);
  template <int i, class T> int f(T const&) {return i;}

  main.cpp
  --
  int main(int, char**) {return f(3.14);}

An error is reported that the value for `i` could not be deduced,
because in the AST -> PCH -> AST conversions the default value `42` is
lost.

I am just now finishing up a patch which adds one more item to the
serialized output for these template types, indicating the decl (with
`Record.AddDeclRef`) inherited from if present.

This seems to be working: fixes the errors seen in my repro (see
bugzilla link above for the repro patch), and the changes seem mostly
clean.  :)

Does this approach sound ok in theory?

Thanks!

--steveo
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: PCH serialization issues (PR 34728)

David Chisnall via cfe-dev
Hi Steve,

On Tue, Sep 26, 2017 at 6:56 PM, Steve O'Brien via cfe-dev
<[hidden email]> wrote:

> I'm running into trouble when serializing a template function
> declaration to PCH and subsequently reading it back:
> https://bugs.llvm.org/show_bug.cgi?id=34728
>
> Small example:
>
>   header.h
>   --
>   template <int i=42, class T> int f(T const&);
>   template <int i, class T> int f(T const&) {return i;}
>
>   main.cpp
>   --
>   int main(int, char**) {return f(3.14);}
>
> An error is reported that the value for `i` could not be deduced,
> because in the AST -> PCH -> AST conversions the default value `42` is
> lost.
>
> I am just now finishing up a patch which adds one more item to the
> serialized output for these template types, indicating the decl (with
> `Record.AddDeclRef`) inherited from if present.
>
> This seems to be working: fixes the errors seen in my repro (see
> bugzilla link above for the repro patch), and the changes seem mostly
> clean.  :)
>
> Does this approach sound ok in theory?

Can't say more without looking at the patch but in theory it's ok.
Worst case we can iterate on a proper fix, please send a patch :-)

-Bruno
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: PCH serialization issues (PR 34728)

David Chisnall via cfe-dev
Yup, sent :)  https://reviews.llvm.org/D38320

On Wed, Sep 27, 2017 at 1:26 PM, Bruno Cardoso Lopes via cfe-dev
<[hidden email]> wrote:

> Hi Steve,
>
> On Tue, Sep 26, 2017 at 6:56 PM, Steve O'Brien via cfe-dev
> <[hidden email]> wrote:
>> I'm running into trouble when serializing a template function
>> declaration to PCH and subsequently reading it back:
>> https://bugs.llvm.org/show_bug.cgi?id=34728
>>
>> Small example:
>>
>>   header.h
>>   --
>>   template <int i=42, class T> int f(T const&);
>>   template <int i, class T> int f(T const&) {return i;}
>>
>>   main.cpp
>>   --
>>   int main(int, char**) {return f(3.14);}
>>
>> An error is reported that the value for `i` could not be deduced,
>> because in the AST -> PCH -> AST conversions the default value `42` is
>> lost.
>>
>> I am just now finishing up a patch which adds one more item to the
>> serialized output for these template types, indicating the decl (with
>> `Record.AddDeclRef`) inherited from if present.
>>
>> This seems to be working: fixes the errors seen in my repro (see
>> bugzilla link above for the repro patch), and the changes seem mostly
>> clean.  :)
>>
>> Does this approach sound ok in theory?
>
> Can't say more without looking at the patch but in theory it's ok.
> Worst case we can iterate on a proper fix, please send a patch :-)
>
> -Bruno
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev