[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added subscribers: dyung, bjope.
bjope added inline comments.
Herald added a subscriber: Charusso.


================
Comment at: clang-tidy/utils/ExceptionAnalyzer.h:112
+    /// throw, if it's unknown or if it won't throw.
+    enum State Behaviour : 2;
+
----------------
(post-commit comments)

I've seen that @dyung did some VS2015 fixes related to this in https://reviews.llvm.org/rCTE354545.

But I think there also is some history of problems with typed enums used in bitfields with GCC (I'm not sure about the details, but it might be discussed here https://reviews.llvm.org/D24289, and there have been workarounds applied before, for example here https://reviews.llvm.org/rCTE319608).

I do not know if we actually have a goal to workaround such problems (no warnings when using GCC), nor do I know if GCC produce correct code despite all the warnings we see with GCC (7.4.0) after this patch, such as

```
../tools/clang/tools/extra/clang-tidy/bugprone/../utils/ExceptionAnalyzer.h:112:23: warning: 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::Behaviour' is too small to hold all values of 'enum class clang::tidy::utils::ExceptionAnalyzer::State'
      State Behaviour : 2;
                        ^
../tools/clang/tools/extra/clang-tidy/bugprone/../utils/ExceptionAnalyzer.h:112:23: warning: 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::Behaviour' is too small to hold all values of 'enum class clang::tidy::utils::ExceptionAnalyzer::State'
     State Behaviour : 2;
                       ^
In file included from ../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:9:0:
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.h:112:23: warning: 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::Behaviour' is too small to hold all values of 'enum class clang::tidy::utils::ExceptionAnalyzer::State'
     State Behaviour : 2;
                       ^
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp: In member function 'clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo& clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::merge(const clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo&)':
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:40:28: warning: comparison is always false due to limited range of data type [-Wtype-limits]
   else if (Other.Behaviour == State::Unknown && Behaviour == State::NotThrowing)
            ~~~~~~~~~~~~~~~~^~~~~~~~
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:41:24: warning: overflow in implicit constant conversion [-Woverflow]
     Behaviour = State::Unknown;
                        ^~~~~~~
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp: In member function 'void clang::tidy::utils::ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour()':
../tools/clang/tools/extra/clang-tidy/utils/ExceptionAnalyzer.cpp:105:26: warning: overflow in implicit constant conversion [-Woverflow]
       Behaviour = State::Unknown;
                          ^~~~~~~
```

To sum up:
The warning are a little bit annoying (but maybe something we have to live with if this is known bugs in gcc).
It seems like we have done other workarounds in the past (writing ugly code to satisfy MSVC and GCC). So should we do that here as well?
Might wanna check if GCC produce correct code for this.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57883/new/

https://reviews.llvm.org/D57883