alexfh requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: clang-tidy/misc/EnumMisuseCheck.cpp:215
@@ +214,3 @@
+ "number(s)");
+ diag(RhsExpr->getExprLoc(), "Used here as a bitmask.",
+ DiagnosticIDs::Note);
----------------
Diagnostic messages are not full sentences, so they shouldn't start with a
capital letter and end with a period.
================
Comment at: clang-tidy/misc/EnumMisuseCheck.h:26
@@ +25,3 @@
+ EnumMisuseCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context), IsStrict(Options.get("IsStrict", 1)) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
----------------
1. Please move the constructor body to the .cpp file so that the code reading
and storing options are together.
2. Let's use a global flag `StrictMode` (used by one more check currently:
http://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html,
clang-tidy/misc/ArgumentCommentCheck.cpp). It can still be configured
separately for each check, but overall it improves consistency. Also, let's
make it non-strict by default.
Options.get(...)
should change to
StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0)
================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:21
@@ +20,3 @@
+
+ 2. Investigating the right hand side of += or |= operator. (only in "Strict")
+ 3. Check only the enum value side of a | or + operator if one of them is not
----------------
Please enclose inline code snippets in backquotes (`+=`, `|=`, etc.). Many
places in this file and in doxygen comments.
================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:29
@@ +28,3 @@
+
+.. code:: c++
+
----------------
Should be `.. code-block:: c++`.
================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:31
@@ +30,3 @@
+
+//1.
+Enum {A, B, C}
----------------
Code block should be indented. Please compile the doc and make sure the result
seems reasonable.
================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:31
@@ +30,3 @@
+
+//1.
+Enum {A, B, C}
----------------
alexfh wrote:
> Code block should be indented. Please compile the doc and make sure the
> result seems reasonable.
> Could you add some descriptions about what 1 stands for here? strict or
> non-strict? please leave a blank between "//" and comment words, the same
> below. Make sure the code here align with code style.
Still not addressed.
================
Comment at: docs/clang-tidy/checks/misc-enum-misuse.rst:38
@@ +37,3 @@
+flag = A | H; //OK, disjoint value intervalls in the enum types > probably
good use
+flag = B | F; //warning, have common values so they are probably misused
+
----------------
> nit: space after //
> here and below.
This is still not addressed.
================
Comment at: test/clang-tidy/misc-enum-misuse-weak.cpp:2
@@ +1,3 @@
+// RUN: %check_clang_tidy %s misc-enum-misuse %t -- -config="{CheckOptions:
[{key: misc-enum-misuse.IsStrict, value: 1}]}" --
+
+enum A { A = 1,
----------------
The format still seems off.
================
Comment at: test/clang-tidy/misc-enum-misuse-weak.cpp:66
@@ +65,3 @@
+ p = A | G;
+ //p = G | X;
+ return 0;
----------------
Is the commented line needed?
================
Comment at: test/clang-tidy/misc-enum-misuse.cpp:2
@@ +1,3 @@
+// RUN: %check_clang_tidy %s misc-enum-misuse %t -- -config="{CheckOptions:
[{key: misc-enum-misuse.IsStrict, value: 0}]}" --
+
+enum Empty {
----------------
Doesn't seem to be done: the format is still off.
https://reviews.llvm.org/D22507
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
