Using "auto <function()> -> returnType" breaks prepare-ChangeLog

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Brady Eidson
I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

JF Bastien-2
https://bugs.webkit.org/show_bug.cgi?id=174930

I’ll trade this C++ fix for ES6 functions getting parsed properly too :D

On Jul 27, 2017, at 23:06, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Sam Weinig-2
In reply to this post by Brady Eidson
For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

It is also the only format available for lambdas, so people are probably getting used to it.

Not sure it’s worth it to avoid it.

- Sam

On Jul 27, 2017, at 11:06 PM, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

JF Bastien-2


On Jul 28, 2017, at 10:29, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

It is also the only format available for lambdas, so people are probably getting used to it.

Not sure it’s worth it to avoid it.

Agreed.

That being said, I’m not volunteering to fix the parser’s handling of lambdas. At that point we should really consider using clang’s AST.


- Sam

On Jul 27, 2017, at 11:06 PM, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Brady Eidson
In reply to this post by Sam Weinig-2

On Jul 28, 2017, at 10:29 AM, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

In this example, yes it is notably shorter.

But neither of these definitions are remotely readable without puttin' on your thinkin' cap.

It is also the only format available for lambdas, so people are probably getting used to it.

Lambdas don't have the problem of prepare-ChangeLog completely missing edits to a member function

Not sure it’s worth it to avoid it.

My request was to be aware of this while prepare-ChangeLog was broken for this case.
Seems like that's almost fixed. :)

~Brady


- Sam

On Jul 27, 2017, at 11:06 PM, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev



_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Sam Weinig-2
In reply to this post by JF Bastien-2


On Jul 28, 2017, at 10:31 AM, JF Bastien <[hidden email]> wrote:



On Jul 28, 2017, at 10:29, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

It is also the only format available for lambdas, so people are probably getting used to it.

Not sure it’s worth it to avoid it.

Agreed.

That being said, I’m not volunteering to fix the parser’s handling of lambdas. At that point we should really consider using clang’s AST.

I absolutely agree. For this and the style checker, it’s probably time to migrate to clang tooling.

- Sam



- Sam

On Jul 27, 2017, at 11:06 PM, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

JF Bastien-2


On Jul 28, 2017, at 10:58, Sam Weinig <[hidden email]> wrote:



On Jul 28, 2017, at 10:31 AM, JF Bastien <[hidden email]> wrote:



On Jul 28, 2017, at 10:29, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

It is also the only format available for lambdas, so people are probably getting used to it.

Not sure it’s worth it to avoid it.

Agreed.

That being said, I’m not volunteering to fix the parser’s handling of lambdas. At that point we should really consider using clang’s AST.

I absolutely agree. For this and the style checker, it’s probably time to migrate to clang tooling.

I think you misspelled “volunteer”.


- Sam



- Sam

On Jul 27, 2017, at 11:06 PM, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Sam Weinig-2
In reply to this post by Brady Eidson


On Jul 28, 2017, at 10:57 AM, Brady Eidson <[hidden email]> wrote:


On Jul 28, 2017, at 10:29 AM, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

In this example, yes it is notably shorter.

But neither of these definitions are remotely readable without puttin' on your thinkin' cap.

It is also the only format available for lambdas, so people are probably getting used to it.

Lambdas don't have the problem of prepare-ChangeLog completely missing edits to a member function

Not sure it’s worth it to avoid it.

My request was to be aware of this while prepare-ChangeLog was broken for this case.
Seems like that's almost fixed. :)

Oh ok. I thought you were proposing a ban.

- Sam


~Brady


- Sam

On Jul 27, 2017, at 11:06 PM, Brady Eidson <[hidden email]> wrote:

I just noticed this tonight. When a change is made to one of these functions, prepare-ChangeLog doesn't log the functions that changed.

I have a question and a request.

Question:

The functions in question where I noticed this:
auto WebURLSchemeTask::didComplete(const ResourceError& error) -> ExceptionType

Would normally be written as:
WebURLSchemeTask::ExceptionType WebURLSchemeTask::didComplete(const ResourceError& error)

Is there some actual value to using this syntax other than… being syntactically different, and being just a little shorter?

Request:

Until we fix prepare-ChangeLog, and unless there is some great advantage to this syntax that I'm not aware of… it's hold off on using it more.

Thanks,
~Brady
_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Maciej Stachowiak
In reply to this post by Sam Weinig-2


On Jul 28, 2017, at 10:58 AM, Sam Weinig <[hidden email]> wrote:



On Jul 28, 2017, at 10:31 AM, JF Bastien <[hidden email]> wrote:



On Jul 28, 2017, at 10:29, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

It is also the only format available for lambdas, so people are probably getting used to it.

Not sure it’s worth it to avoid it.

Agreed.

That being said, I’m not volunteering to fix the parser’s handling of lambdas. At that point we should really consider using clang’s AST.

I absolutely agree. For this and the style checker, it’s probably time to migrate to clang tooling.

Using a real parser is probably a good idea at some point.

But I am not sure it's necessary for prepare-ChangeLog to know about the boundaries for lambdas. It's goal is to list all named functions that the local changes have modified. For modifications to the body or signature of a lambda, naming the function that contains the lambda is probably the most useful option. I think that already works as expected.

Regards,
Maciej




_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Using "auto <function()> -> returnType" breaks prepare-ChangeLog

Sam Weinig-2


On Jul 28, 2017, at 11:13 AM, Maciej Stachowiak <[hidden email]> wrote:



On Jul 28, 2017, at 10:58 AM, Sam Weinig <[hidden email]> wrote:



On Jul 28, 2017, at 10:31 AM, JF Bastien <[hidden email]> wrote:



On Jul 28, 2017, at 10:29, Sam Weinig <[hidden email]> wrote:

For some generic programming, this form can be dramatically shorter:

e.g. 

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE auto HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value) -> AddResult
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

vs.

template<typename KeyArg, typename MappedArg, typename HashArg, typename KeyTraitsArg, typename MappedTraitsArg>
template<typename K, typename V>
ALWAYS_INLINE typename HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::AddResult HashMap<KeyArg, MappedArg, HashArg, KeyTraitsArg, MappedTraitsArg>::inlineAdd(K&& key, V&& value)
{
    return m_impl.template add<HashMapTranslator<KeyValuePairTraits, HashFunctions>>(std::forward<K>(key), std::forward<V>(value));
}

It is also the only format available for lambdas, so people are probably getting used to it.

Not sure it’s worth it to avoid it.

Agreed.

That being said, I’m not volunteering to fix the parser’s handling of lambdas. At that point we should really consider using clang’s AST.

I absolutely agree. For this and the style checker, it’s probably time to migrate to clang tooling.

Using a real parser is probably a good idea at some point.

But I am not sure it's necessary for prepare-ChangeLog to know about the boundaries for lambdas. It's goal is to list all named functions that the local changes have modified. For modifications to the body or signature of a lambda, naming the function that contains the lambda is probably the most useful option. I think that already works as expected.


I didn’t mean to imply that we should have lambdas in ChangeLogs, only that people were probably getting used to that syntax. Sorry for the confusion.

-Sam


_______________________________________________
webkit-dev mailing list
[hidden email]
https://lists.webkit.org/mailman/listinfo/webkit-dev
Loading...