Rename all shadowed types and variables and enable Wshadow when in pedantic mode #1965
Conversation
jgopel
force-pushed
the
enable-wshadow
branch
from
d1f3483
to
609c5fb
Compare
include/fmt/os.h
Outdated
| ostream_params(T... params, int oflag) : ostream_params(params...) { | ||
| this->oflag = oflag; | ||
| ostream_params(T... params, int new_oflag) : ostream_params(params...) { | ||
| this->oflag = new_oflag; |
test/core-test.cc
Outdated
| @@ -213,12 +213,12 @@ struct custom_context { | |||
|
|
|||
| template <typename T> struct formatter_type { | |||
| template <typename ParseContext> | |||
| auto parse(ParseContext& ctx) -> decltype(ctx.begin()) { | |||
| return ctx.begin(); | |||
| auto parse(ParseContext& parse_ctx) -> decltype(parse_ctx.begin()) { | |||
There was a problem hiding this comment.
Let's keep argument names as is but rename the data member to parse_ctx since it has wider scope.
There was a problem hiding this comment.
Done - I'm glad you looked at that one, I had a feeling that the situation might be something like this but I didn't want to be too aggressive so I tried my best to keep the scope very local.
test/format-test.cc
Outdated
| @@ -219,12 +219,12 @@ TEST(AllocatorTest, allocator_ref) { | |||
| check_forwarding(alloc, ref3); | |||
| } | |||
|
|
|||
| typedef allocator_ref<std::allocator<char>> TestAllocator; | |||
| typedef allocator_ref<std::allocator<char>> StdAllocator; | |||
There was a problem hiding this comment.
Let's follow the naming conventions while at it:
using std_allocator = allocator_ref<std::allocator<char>>;
jgopel
force-pushed
the
enable-wshadow
branch
4 times, most recently
from
187b3b0
to
9cc2342
Compare
|
A few more build failures: https://travis-ci.org/github/fmtlib/fmt/jobs/740386845 |
jgopel
force-pushed
the
enable-wshadow
branch
from
9cc2342
to
87b4d2a
Compare
|
I can't get this last set of errors to reproduce locally for reasons that are not clear to me, so there might be a bit of churn with using CI to find the errors. |
jgopel
force-pushed
the
enable-wshadow
branch
2 times, most recently
from
51fbcd4
to
40dab6e
Compare
|
Just in case anyone else encounters the issue of not being able to reproduce errors locally - the C++ standard seems to matter to at least GCC for whether or not |
test/format-test.cc
Outdated
| s = fmt::detail::bit_cast<uint32_pair>(uint64_t(~0ull)); | ||
| s = fmt::detail::bit_cast<uint32_pair>(~0ull); |
There was a problem hiding this comment.
This looks unrelated and technically not correct.
There was a problem hiding this comment.
Whoops, sorry about that - fixed.
test/std-format-test.cc
Outdated
| constexpr auto parse(format_parse_context& ctx) { | ||
| auto parse(format_parse_context& ctx) { |
There was a problem hiding this comment.
Whoops, sorry about that - fixed
Problem: - All `-Wshadow` warnings are fixed but there is nothing stopping them from being reintroduced. Solution: - Fail pedantic builds on `-Wshadow` warnings. This allows CI to prevent reoccurrence of the warning. Notes: - Not enabling `-Wshadow` for gcc versions 4 or lower because the warning is much more aggressive there to the point that it's mostly just noise.
jgopel
force-pushed
the
enable-wshadow
branch
from
40dab6e
to
82f6af4
Compare
|
@jgopel thank you for doing this. It was annoying my team for some time now :) |
|
Merged, thanks! |
I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.