Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang 7.0 support - individual warning levels #100

Merged
merged 110 commits into from
Feb 4, 2018

Conversation

gabyx
Copy link
Contributor

@gabyx gabyx commented Jan 20, 2018

this not a really good pull request, I started it, to keep working on these c++17 warnings...

  • there are two warnings regarding array bound checks somewhere in the catch.hpp dependency (think its marco expansion?)
/Users/gabrielnuetzi/Desktop/rttr/src/rttr/../rttr/detail/impl/array_range_impl.h: In function 'void ____C_A_T_C_H____T_E_S_T____139()':
/Users/gabrielnuetzi/Desktop/rttr/src/rttr/../rttr/detail/impl/array_range_impl.h:200:105: warning: array subscript is below array bounds [-Warray-bounds]
     return (empty_() ? const_reverse_iterator{m_begin, this} : const_reverse_iterator{m_begin - 1, this});
  • lots of no-except warnings:
/Users/gabrielnuetzi/Desktop/rttr/src/rttr/../rttr/detail/registration/registration_impl.h:205:60: error: mangled name for 'rttr::registration::bind<rttr::detail::meth, Class_Type, F, acc_level> rttr::registration::class_<Class_Type>::method(rttr::string_view, F, acc_level) [with F = long unsigned int (std::__cxx11::basic_string<char>::*)() const noexcept; acc_level = rttr::detail::public_access; Class_Type = std::__cxx11::basic_string<char>]' will change in C++17 because the exception specification is part of a function type [-Werror=noexcept-type]
 registration::bind<detail::meth, Class_Type, F, acc_level> registration::class_<Class_Type>::method(string_view name, F f, acc_level level)

@acki-m
Copy link
Contributor

acki-m commented Jan 20, 2018

You should merge my pullrequest changes into yours. So we can see whether the compile errors are gone.
I think it is better you find a solution, where the warnings are gone, without removing werror.
What do you think?

@coveralls
Copy link

coveralls commented Jan 20, 2018

Coverage Status

Coverage increased (+0.1%) to 90.235% when pulling d0e9083 on gabyx:disable-warnings into 29e6ff9 on rttrorg:master.

@gabyx
Copy link
Contributor Author

gabyx commented Jan 20, 2018

jeah of course!! :-) but I have not so much time fixing these now (also I am not really experienced in these errors...), but of course one should fix them (maybe you could point out some places to look at or how to potentially fix them?)
I just made a branch to build without -Werror... such that it compiles...

to be continued...

@acki-m
Copy link
Contributor

acki-m commented Jan 21, 2018

The thing is, when somebody is using RTTR and he likes to user -Wall and -Werror, he cannot use RTTR.
because he will get compile errors, because the problematic code are in the class templates.

So your changes will not help here. You have to find a way to correctly fix the code or disable the warning directly in the header files.
When you can provide enough info to me, that we cannot fix the code and have to disable the warning, then I would merge your PR.

@gabyx
Copy link
Contributor Author

gabyx commented Jan 21, 2018

Thats true, my fix is only a stupid hack at the moment...

acki-m and others added 18 commits February 1, 2018 22:43
error C2752: 'template_type_trait<std::array<bool, 100>>': more than one partial specialization matches the template argument list
note: could be 'template_type_trait<T<N>>'
note: or       'template_type_trait<T<N>>'

Reported to MS
The error msg was:
_Pragma ("clang diagnostic ignored \"-Wnoexcept-type\"")

The version scheme for clang on MacOSX is different then for the other operating systems.
The version prior clang 9 on mac did not support this pragma. So we disable it.

thx to @gabyx
@gabyx gabyx mentioned this pull request Feb 4, 2018
@acki-m
Copy link
Contributor

acki-m commented Feb 4, 2018

@gabyx
I will merge this now.
thx

@acki-m acki-m merged commit 3ee7fd7 into rttrorg:master Feb 4, 2018
@gabyx
Copy link
Contributor Author

gabyx commented Feb 4, 2018

thx too for your efforts

@acki-m acki-m mentioned this pull request Feb 4, 2018
@gabyx gabyx deleted the disable-warnings branch February 16, 2018 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants