format: add checks for type aliases#12099
Conversation
Signed-off-by: tomocy <tomocy.dev@gmail.com>
| } | ||
|
|
||
| USING_TYPE_ALIAS_REGEX = re.compile("(using .* = .*;|typedef .* .*;)") | ||
| SMART_PTR_REGEX = re.compile("std::(unique_ptr|shared_ptr)<(.*?)>(?!;)") |
There was a problem hiding this comment.
Should we introduce type alias for weak_ptr? This kind of smart pointer is partly used in envoyproxy/envoy and envoy/envoy-wasm. WDYT? cc. @jmarantz
There was a problem hiding this comment.
Sure, but I'd recommend not expanding the scope of this series of PRs. Would be an easier follow-up, as there are relatively fewer weak_ptr uses I think.
jmarantz
left a comment
jmarantz
left a comment
There was a problem hiding this comment.
I have a high level question about the enforcement of this. It is very nice to use SharedPtr and ScopedPtr abbreviations. However, there is at least one scenario where spelling out the template is much better, even if a nickname exists, which is when the usage is templated.
I'm actually working on such a scenario now in https://github.com/envoyproxy/envoy/pull/12128/files though it uses RefcountPtr<T> rather than shared_ptr<T>.
There's also existing templatized code in
which -- if this guideline was enforced with aformat check, would need a ton of exceptions.
| help="specify the header block include directory order.") | ||
| parser.add_argument("--aggressive", | ||
| action='store_true', | ||
| help="specify if it fixes formats making risky changes.") |
There was a problem hiding this comment.
help="makes risky format changes; re-testing needed after making aggressive fixes"
|
Please see @snowp's comment in slack: https://envoyproxy.slack.com/archives/C78HA81DH/p1594908964168900 I'm not sure that deep-link works properly but it's in #envoy-dev and @snowp wrote it on July 16th, and it starts with "just a thought i had: would it make sense to be using templated " |
|
/wait |
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
3 participants
Signed-off-by: tomocy tomocy.dev@gmail.com
Commit Message: format: add checks for type aliases
Additional Description: N/A
Risk Level: Low
Testing: N/A
Docs Changes: N/A
Release Notes: N/A
Part of #11634