aaron.ballman added a comment.
Sorry about the delayed review on this -- I had intended to provide comments
earlier, but somehow this fell off my radar.
================
Comment at: docs/ReleaseNotes.rst:53
+- ``-Wextra-semi-stmt`` is a new diagnostic, which is, much like
+ ``-Wextra-semi``, diagnoses extra semicolons. This new diagnostic
----------------
, which is, much like `-Wextra-semi`, diagnoses extra semicolons. -> that
diagnoses extra semicolons, much like `-Wextra-semi`.
================
Comment at: docs/ReleaseNotes.rst:55
+ ``-Wextra-semi``, diagnoses extra semicolons. This new diagnostic
+ fires on all *unnecessary* null statements (expression statements without
+ an expression), unless: the semi directly follows a macro that was
----------------
fires on all -> diagnoses
================
Comment at: docs/ReleaseNotes.rst:56
+ fires on all *unnecessary* null statements (expression statements without
+ an expression), unless: the semi directly follows a macro that was
+ expanded to nothing; the semi is within the macro itself (both macros from
----------------
semi -> semicolon
================
Comment at: docs/ReleaseNotes.rst:57
+ an expression), unless: the semi directly follows a macro that was
+ expanded to nothing; the semi is within the macro itself (both macros from
+ system headers, and normal macros).
----------------
nothing; the semi is -> nothing or if the semicolon
I'd make the parenthetical a stand-alone sentence. Something like: "This
applies to macros defined in system headers as well as user-defined macros."
================
Comment at: docs/ReleaseNotes.rst:65-68
+ struct S {
+ int *begin();
+ int *end();
+ };
----------------
I'd drop this from the example.
================
Comment at: docs/ReleaseNotes.rst:91
+
+- ``-Wempty-init-stmt`` is a new diagnostic, which is diagnoses empty
+ init-statements of ``if``, ``switch``, ``range-based for``, unless: the semi
----------------
diagnostic, which is diagnoses empty -> diagnostic that diagnoses empty
================
Comment at: docs/ReleaseNotes.rst:92
+- ``-Wempty-init-stmt`` is a new diagnostic, which is diagnoses empty
+ init-statements of ``if``, ``switch``, ``range-based for``, unless: the semi
+ directly follows a macro that was expanded to nothing; the semi is within the
----------------
semi -> semicolon
================
Comment at: docs/ReleaseNotes.rst:93-94
+ init-statements of ``if``, ``switch``, ``range-based for``, unless: the semi
+ directly follows a macro that was expanded to nothing; the semi is within the
+ macro itself (both macros from system headers, and normal macros).
+ This diagnostic in ``-Wextra-semi-stmt`` group, and is enabled in
``-Wextra``.
----------------
nothing; the semi is -> nothing or if the semicolon
I'd make the parenthetical a stand-alone sentence. Something like: "This
applies to macros defined in system headers as well as user-defined macros."
================
Comment at: docs/ReleaseNotes.rst:95
+ macro itself (both macros from system headers, and normal macros).
+ This diagnostic in ``-Wextra-semi-stmt`` group, and is enabled in
``-Wextra``.
+
----------------
diagnostic in -> diagnostic is in the
Drop the comma.
================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:57-58
+def warn_null_statement : Warning<
+ "statement has no effect, ';' "
+ "with no preceding expression is a null statement">,
+ InGroup<ExtraSemiStmt>, DefaultIgnore;
----------------
How about: `empty expression statement has no effect; remove unnecessary ';' to
silence this warning` ?
================
Comment at: include/clang/Basic/DiagnosticParseKinds.td:557-558
+def warn_empty_init_statement : Warning<
+ "init-statement of '%select{if|switch|range-based for}0' is a "
+ "null statement">, InGroup<EmptyInitStatement>, DefaultIgnore;
----------------
How about: `empty initialization statement of '%select{if|switch|range-based
for}0' has no effect`?
================
Comment at: test/Parser/extra-semi-resulting-in-nullstmt.cpp:81
+ }
+}
----------------
There are four additional tests I'd like to see: 1) a semicolon at global
scope, 2) a semicolon after a namespace declaration (e.g., `namespace foo
{};`), 3) a `[[fallthrough]];` null statement, 4) a `[[]];` null statement.
Repository:
rC Clang
https://reviews.llvm.org/D52695
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
