mapbox / mapbox/variant

Fixing clang-tidy errors/warnings

Open
#158 9 comments 0 reactions 1 assignee View on GitHub

@artemp is already working on this.

Since Aug 21, 2017.

Dominant language
C++
Stars
384
Forks
96
PR merge metrics
No merged PRs in 30d

Description

I'm attempting to set up clang-tidy to run on codebases using `variant`. I am consistently seeing most warnings coming from variant. So these need to be addressed here, at the source, by either adding `// NOLINT` where appropriate or fixing.

@artemp can you please tackle this next week?

Here are the warnings:

```
./include/mapbox/variant.hpp:344:16: warning: Function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
return f(unwrapper::apply(v.template get_unchecked()));
^
test/lambda_overload_test.cpp:122:5: note: Calling 'test_match_singleton'
test_match_singleton();
^
test/lambda_overload_test.cpp:87:5: note: Calling 'variant::match'
singleton.match([](int) {});
^
./include/mapbox/variant.hpp:903:16: note: Calling 'variant::visit'
return variant::visit(*this, ::mapbox::util::make_visitor(std::forward(fs)...));
^
./include/mapbox/variant.hpp:871:16: note: Calling 'dispatcher::apply'
return detail::dispatcher::apply(v, std::forward(f));
^
./include/mapbox/variant.hpp:344:18: note: Calling 'unwrapper::apply'
return f(unwrapper::apply(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:344:18: note: Returning from 'unwrapper::apply'
return f(unwrapper::apply(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:344:16: note: Function call argument is an uninitialized value
return f(unwrapper::apply(v.template get_unchecked()));
^
2 warnings generated.
./include/mapbox/recursive_wrapper.hpp:106:36: warning: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn]
const T* get_pointer() const { return p_; }
^
test/hashable_test.cpp:157:5: note: Calling 'test_recursive_hashable'
test_recursive_hashable();
^
test/hashable_test.cpp:142:10: note: Calling move constructor for 'variant'
Tree tree = Node{Node{Empty{}, Empty{}}, Empty{}};
^
./include/mapbox/variant.hpp:615:9: note: Calling 'variant_helper::move'
helper_type::move(old.type_index, &old.data, &data);
^
./include/mapbox/variant.hpp:235:9: note: Taking false branch
if (old_type_index == sizeof...(Types))
^
./include/mapbox/variant.hpp:241:13: note: Calling 'variant_helper::move'
variant_helper::move(old_type_index, old_value, new_value);
^
./include/mapbox/variant.hpp:235:9: note: Taking false branch
if (old_type_index == sizeof...(Types))
^
./include/mapbox/variant.hpp:241:13: note: Calling 'variant_helper::move'
variant_helper::move(old_type_index, old_value, new_value);
^
./include/mapbox/variant.hpp:241:13: note: Returning from 'variant_helper::move'
variant_helper::move(old_type_index, old_value, new_value);
^
./include/mapbox/variant.hpp:241:13: note: Returning from 'variant_helper::move'
variant_helper::move(old_type_index, old_value, new_value);
^
./include/mapbox/variant.hpp:615:9: note: Returning from 'variant_helper::move'
helper_type::move(old.type_index, &old.data, &data);
^
test/hashable_test.cpp:142:10: note: Returning from move constructor for 'variant'
Tree tree = Node{Node{Empty{}, Empty{}}, Empty{}};
^
test/hashable_test.cpp:144:9: note: Calling 'hash::operator()'
if (std::hash{}(tree) != ((5 + (5 + (3 + 3))) + 3))
^
./include/mapbox/variant.hpp:1022:16: note: Calling 'apply_visitor'
return ::mapbox::util::apply_visitor(::mapbox::util::detail::hasher{}, v);
^
./include/mapbox/variant.hpp:959:12: note: Calling 'variant::visit'
return V::visit(v, std::forward(f));
^
./include/mapbox/variant.hpp:864:16: note: Calling 'dispatcher::apply_const'
return detail::dispatcher::apply_const(v, std::forward(f));
^
./include/mapbox/variant.hpp:311:9: note: Taking false branch
if (v.template is())
^
./include/mapbox/variant.hpp:317:20: note: Calling 'dispatcher::apply_const'
return dispatcher::apply_const(v, std::forward(f));
^
./include/mapbox/variant.hpp:339:18: note: Calling 'unwrapper::apply_const'
return f(unwrapper::apply_const(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:279:16: note: Calling 'recursive_wrapper::get'
return obj.get();
^
./include/mapbox/recursive_wrapper.hpp:101:17: note: Calling 'recursive_wrapper::get_pointer'
return *get_pointer();
^
./include/mapbox/recursive_wrapper.hpp:106:36: note: Undefined or garbage value returned to caller
const T* get_pointer() const { return p_; }
^
./include/mapbox/variant.hpp:555:16: warning: Function call argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
return std::hash{}(hashable);
^
test/hashable_test.cpp:153:5: note: Calling 'test_singleton'
test_singleton();
^
test/hashable_test.cpp:17:7: note: Calling move constructor for 'variant'
V singleton = 5;
^
./include/mapbox/variant.hpp:615:9: note: Calling 'variant_helper::move'
helper_type::move(old.type_index, &old.data, &data);
^
./include/mapbox/variant.hpp:235:9: note: Taking false branch
if (old_type_index == sizeof...(Types))
^
./include/mapbox/variant.hpp:241:13: note: Calling 'variant_helper::move'
variant_helper::move(old_type_index, old_value, new_value);
^
./include/mapbox/variant.hpp:241:13: note: Returning from 'variant_helper::move'
variant_helper::move(old_type_index, old_value, new_value);
^
./include/mapbox/variant.hpp:615:9: note: Returning from 'variant_helper::move'
helper_type::move(old.type_index, &old.data, &data);
^
test/hashable_test.cpp:17:7: note: Returning from move constructor for 'variant'
V singleton = 5;
^
test/hashable_test.cpp:19:9: note: Calling 'hash::operator()'
if (std::hash{}(singleton) != std::hash{}(5))
^
./include/mapbox/variant.hpp:1022:16: note: Calling 'apply_visitor'
return ::mapbox::util::apply_visitor(::mapbox::util::detail::hasher{}, v);
^
./include/mapbox/variant.hpp:959:12: note: Calling 'variant::visit'
return V::visit(v, std::forward(f));
^
./include/mapbox/variant.hpp:864:16: note: Calling 'dispatcher::apply_const'
return detail::dispatcher::apply_const(v, std::forward(f));
^
./include/mapbox/variant.hpp:339:18: note: Calling 'unwrapper::apply_const'
return f(unwrapper::apply_const(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:339:18: note: Returning from 'unwrapper::apply_const'
return f(unwrapper::apply_const(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:339:18: note: Passing value via 1st parameter 'hashable'
return f(unwrapper::apply_const(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:339:16: note: Calling 'hasher::operator()'
return f(unwrapper::apply_const(v.template get_unchecked()));
^
./include/mapbox/variant.hpp:555:16: note: Function call argument is an uninitialized value
return std::hash{}(hashable);
```

Here is how I'm testing:

- ` run-clang-tidy` branch: https://github.com/mapbox/variant/compare/run-clang-tidy
- Running `./scripts/clang-tidy.sh`

/cc @daniel-j-h

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.