On 02/26/2014 10:27 PM, Tobias Grosser wrote:
On 02/26/2014 10:19 PM, Chris Lattner wrote:
On Feb 26, 2014, at 2:22 AM, Tobias Grosser <[email protected]> wrote:
2) Adding the new severity level / the name of the diagnostic
Only small issues have been found in the patch. All of them have been
addressed. The last open issues was the name of the diagnostic. Richard
proposed 'info' or 'remark'. Chris and Eric prefer to call the
severity 'info', in case there is no prior art. However, Alexander
and Arthur mentioned prior art for 'remark' in both icc and edg. Also
the comment from Arthur sounds right:
I don't know of any compiler that uses the term "informative".
Besides, that's redundant; *all* compiler diagnostics are purely
"informative" by definition. The variable here is //severity//:
fatal-error, recoverable-error, warning, remark, silent.
I personally preferred 'info' first, but now came to the conclusion
that 'remark' is the better option, except someone sees strong
reasons to ignore the prior art.
“remark” is fine with me.
Good. I will submit patches accordingly.
3) How to enable 'remarks'
We need a way to enable 'remark' diagnostics. Quentin proposed to go
for an approach similar to the warning flags. Where we control remarks
with '-Rvector', '-Rloop-vector', ...
I will read a little bit through the existing option system to better
understand what it is doing, possibly adding documentation / cleanups
on my way. I will come back with a proposal here.
It’s a bit odd, but since these are diagnostics, why not use the
existing -W flags? You should be able to -Werror one of these,
control them with #pragma clang diagnostics, etc. It doesn’t seem
like we need more complexity in this space.
Good point. I will prepare the above patches such that they reuse the
existing infrastructure. If we really see a need for further
adjustments, we can do this incrementally.
I just updated the patch to reflect the conclusions of our discussion.
Please review for commit.
------------
[PATCH] Add 'remark' diagnostic type in 'clang'
A 'remark' is information that is not an error or a warning, but rather
some additional information provided to the user. In contrast to a
'note' a 'remark' is an independent diagnostic, whereas a 'note' always
depends on another diagnostic.
A typical use case for remark nodes is information provided to the user,
e.g. information provided by the vectorizer about loops that have been
vectorized.
This patch provides the initial implementation of 'remarks'. It includes
the actual definiton of the remark nodes, their printing as well as
basic parameter handling. We are reusing the existing diagnostic
parameters which means a remark
can be enabled with normal '-Wdiagnostic-name' flags and can be upgraded
to an error using '-Werror=diagnostic-name'.
This patch is by intention minimal in terms of parameter handling. More
experience and more discussions will most likely lead to further
enhancements in the parameter handling.
------------
Thanks,
Tobias
>From b2281a0807f44ff67d4144a1769d566871c0c1b3 Mon Sep 17 00:00:00 2001
From: Tobias Grosser <[email protected]>
Date: Tue, 25 Feb 2014 09:37:24 +0100
Subject: [PATCH] Add 'remark' diagnostic type in 'clang'
A 'remark' is information that is not an error or a warning, but rather some
additional information provided to the user. In contrast to a 'note' a 'remark'
is an independent diagnostic, whereas a 'note' always depends on another
diagnostic.
A typical use case for remark nodes is information provided to the user, e.g.
information provided by the vectorizer about loops that have been vectorized.
This patch provides the initial implementation of 'remarks'. It includes the
actual definiton of the remark nodes, their printing as well as basic parameter
handling. We are reusing the existing diagnostic parameters which means a remark
can be enabled with normal '-Wdiagnostic-name' flags and can be upgraded to
an error using '-Werror=diagnostic-name'.
This patch is by intention minimal in terms of parameter handling. More
experience and more discussions will most likely lead to further enhancements
in the parameter handling.
---
docs/InternalsManual.rst | 13 +++++++++----
docs/UsersManual.rst | 12 +++++++-----
include/clang-c/Index.h | 6 ++++++
include/clang/Basic/Diagnostic.h | 1 +
include/clang/Basic/Diagnostic.td | 3 +++
include/clang/Basic/DiagnosticFrontendKinds.td | 1 +
include/clang/Basic/DiagnosticGroups.td | 1 +
include/clang/Basic/DiagnosticIDs.h | 13 +++++++------
lib/Basic/DiagnosticIDs.cpp | 15 ++++++++++++---
lib/CodeGen/CodeGenAction.cpp | 23 ++++++++++++++++++++++-
lib/Frontend/LogDiagnosticPrinter.cpp | 1 +
lib/Frontend/PrintPreprocessedOutput.cpp | 3 +++
lib/Frontend/TextDiagnostic.cpp | 4 ++++
tools/c-index-test/c-index-test.c | 1 +
tools/diagtool/ShowEnabledWarnings.cpp | 1 +
tools/libclang/CIndexDiagnostic.cpp | 1 +
tools/libclang/CXStoredDiagnostic.cpp | 1 +
17 files changed, 81 insertions(+), 19 deletions(-)
diff --git a/docs/InternalsManual.rst b/docs/InternalsManual.rst
index a34ba29..2da640e 100644
--- a/docs/InternalsManual.rst
+++ b/docs/InternalsManual.rst
@@ -87,7 +87,8 @@ start with ``err_``, ``warn_``, ``ext_`` to encode the
severity into the name.
Since the enum is referenced in the C++ code that produces the diagnostic, it
is somewhat useful for it to be reasonably short.
-The severity of the diagnostic comes from the set {``NOTE``, ``WARNING``,
+The severity of the diagnostic comes from the set {``NOTE``, ``REMARK``,
+``WARNING``,
``EXTENSION``, ``EXTWARN``, ``ERROR``}. The ``ERROR`` severity is used for
diagnostics indicating the program is never acceptable under any circumstances.
When an error is emitted, the AST for the input code may not be fully built.
@@ -97,11 +98,15 @@ represent them in the AST, but we produce diagnostics to
tell the user their
code is non-portable. The difference is that the former are ignored by
default, and the later warn by default. The ``WARNING`` severity is used for
constructs that are valid in the currently selected source language but that
-are dubious in some way. The ``NOTE`` level is used to staple more information
-onto previous diagnostics.
+are dubious in some way. The ``REMARK`` severity provides generic
+information about the compilation that is not necessarily
+related to any dubious code.
+The ``NOTE`` level is used
+to staple more information onto previous diagnostics.
These *severities* are mapped into a smaller set (the ``Diagnostic::Level``
-enum, {``Ignored``, ``Note``, ``Warning``, ``Error``, ``Fatal``}) of output
+enum, {``Ignored``, ``Note``, ``Remark``, ``Warning``, ``Error``, ``Fatal``})
of
+output
*levels* by the diagnostics subsystem based on various configuration options.
Clang internally supports a fully fine grained mapping mechanism that allows
you to map almost any diagnostic to the output level that you want. The only
diff --git a/docs/UsersManual.rst b/docs/UsersManual.rst
index 4ca8435..ba91501 100644
--- a/docs/UsersManual.rst
+++ b/docs/UsersManual.rst
@@ -112,11 +112,11 @@ Options to Control Error and Warning Messages
.. option:: -w
- Disable all warnings.
+ Disable all diagnostics.
.. option:: -Weverything
- :ref:`Enable all warnings. <diagnostics_enable_everything>`
+ :ref:`Enable all diagnostics. <diagnostics_enable_everything>`
.. option:: -pedantic
@@ -582,6 +582,7 @@ All diagnostics are mapped into one of these 5 classes:
- Ignored
- Note
+- Remark
- Warning
- Error
- Fatal
@@ -722,11 +723,12 @@ is treated as a system header.
.. _diagnostics_enable_everything:
-Enabling All Warnings
-^^^^^^^^^^^^^^^^^^^^^
+Enabling All Diagnostics
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
In addition to the traditional ``-W`` flags, one can enable **all**
-warnings by passing :option:`-Weverything`. This works as expected with
+diagnostics by passing :option:`-Weverything`. This works as expected
+with
:option:`-Werror`, and also includes the warnings from :option:`-pedantic`.
Note that when combined with :option:`-w` (which disables all warnings), that
diff --git a/include/clang-c/Index.h b/include/clang-c/Index.h
index b77ddcb..7eff0a4 100644
--- a/include/clang-c/Index.h
+++ b/include/clang-c/Index.h
@@ -653,6 +653,12 @@ enum CXDiagnosticSeverity {
CXDiagnostic_Note = 1,
/**
+ * \brief This diagnostic is a remark that provides additional information
+ * for the user.
+ */
+ CXDiagnostic_Remark = 5,
+
+ /**
* \brief This diagnostic indicates suspicious code that may not be
* wrong.
*/
diff --git a/include/clang/Basic/Diagnostic.h b/include/clang/Basic/Diagnostic.h
index 6782964..d83c4c0 100644
--- a/include/clang/Basic/Diagnostic.h
+++ b/include/clang/Basic/Diagnostic.h
@@ -142,6 +142,7 @@ public:
enum Level {
Ignored = DiagnosticIDs::Ignored,
Note = DiagnosticIDs::Note,
+ Remark = DiagnosticIDs::Remark,
Warning = DiagnosticIDs::Warning,
Error = DiagnosticIDs::Error,
Fatal = DiagnosticIDs::Fatal
diff --git a/include/clang/Basic/Diagnostic.td
b/include/clang/Basic/Diagnostic.td
index 2616548..61a4508 100644
--- a/include/clang/Basic/Diagnostic.td
+++ b/include/clang/Basic/Diagnostic.td
@@ -15,6 +15,7 @@
// Define the diagnostic mappings.
class DiagMapping;
def MAP_IGNORE : DiagMapping;
+def MAP_REMARK : DiagMapping;
def MAP_WARNING : DiagMapping;
def MAP_ERROR : DiagMapping;
def MAP_FATAL : DiagMapping;
@@ -22,6 +23,7 @@ def MAP_FATAL : DiagMapping;
// Define the diagnostic classes.
class DiagClass;
def CLASS_NOTE : DiagClass;
+def CLASS_REMARK : DiagClass;
def CLASS_WARNING : DiagClass;
def CLASS_EXTENSION : DiagClass;
def CLASS_ERROR : DiagClass;
@@ -84,6 +86,7 @@ class AccessControl {
// FIXME: ExtWarn and Extension should also be SFINAEFailure by default.
class Error<string str> : Diagnostic<str, CLASS_ERROR, MAP_ERROR>,
SFINAEFailure;
class Warning<string str> : Diagnostic<str, CLASS_WARNING, MAP_WARNING>;
+class Remark<string str> : Diagnostic<str, CLASS_REMARK, MAP_IGNORE>;
class Extension<string str> : Diagnostic<str, CLASS_EXTENSION, MAP_IGNORE>;
class ExtWarn<string str> : Diagnostic<str, CLASS_EXTENSION, MAP_WARNING>;
class Note<string str> : Diagnostic<str, CLASS_NOTE,
MAP_FATAL/*ignored*/>;
diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td
b/include/clang/Basic/DiagnosticFrontendKinds.td
index 2b36d38..41a83a0 100644
--- a/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -29,6 +29,7 @@ def note_fe_backend_frame_larger_than: Note<"%0">, CatBackend;
def warn_fe_backend_plugin: Warning<"%0">, CatBackend, InGroup<BackendPlugin>;
def err_fe_backend_plugin: Error<"%0">, CatBackend;
+def remark_fe_backend_plugin: Remark<"%0">, CatBackend,
InGroup<RemarkBackendPlugin>;
def note_fe_backend_plugin: Note<"%0">, CatBackend;
def err_fe_invalid_code_complete_file : Error<
diff --git a/include/clang/Basic/DiagnosticGroups.td
b/include/clang/Basic/DiagnosticGroups.td
index 12bf163..269be4c 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/include/clang/Basic/DiagnosticGroups.td
@@ -636,3 +636,4 @@ def SourceUsesOpenMP : DiagGroup<"source-uses-openmp">;
def BackendInlineAsm : DiagGroup<"inline-asm">;
def BackendFrameLargerThan : DiagGroup<"frame-larger-than">;
def BackendPlugin : DiagGroup<"backend-plugin">;
+def RemarkBackendPlugin : DiagGroup<"remark-backend-plugin">;
diff --git a/include/clang/Basic/DiagnosticIDs.h
b/include/clang/Basic/DiagnosticIDs.h
index 0152f25..737a3bb 100644
--- a/include/clang/Basic/DiagnosticIDs.h
+++ b/include/clang/Basic/DiagnosticIDs.h
@@ -56,16 +56,17 @@ namespace clang {
};
/// Enum values that allow the client to map NOTEs, WARNINGs, and
EXTENSIONs
- /// to either MAP_IGNORE (nothing), MAP_WARNING (emit a warning), MAP_ERROR
- /// (emit as an error). It allows clients to map errors to
- /// MAP_ERROR/MAP_DEFAULT or MAP_FATAL (stop emitting diagnostics after
this
- /// one).
+ /// to either MAP_IGNORE (nothing), MAP_WARNING (emit a warning),
MAP_REMARK
+ /// (emit a remark), MAP_ERROR (emit as an error). It allows clients to
map
+ /// errors to MAP_ERROR/MAP_DEFAULT or MAP_FATAL (stop emitting diagnostics
+ /// after this one).
enum Mapping {
// NOTE: 0 means "uncomputed".
MAP_IGNORE = 1, ///< Map this diagnostic to nothing, ignore it.
MAP_WARNING = 2, ///< Map this diagnostic to a warning.
MAP_ERROR = 3, ///< Map this diagnostic to an error.
- MAP_FATAL = 4 ///< Map this diagnostic to a fatal error.
+ MAP_REMARK = 4, ///< Map this diagnostic to a remark.
+ MAP_FATAL = 5 ///< Map this diagnostic to a fatal error.
};
}
@@ -113,7 +114,7 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
public:
/// \brief The level of the diagnostic, after it has been through mapping.
enum Level {
- Ignored, Note, Warning, Error, Fatal
+ Ignored, Note, Remark, Warning, Error, Fatal
};
private:
diff --git a/lib/Basic/DiagnosticIDs.cpp b/lib/Basic/DiagnosticIDs.cpp
index c8cd03e..cf6933a 100644
--- a/lib/Basic/DiagnosticIDs.cpp
+++ b/lib/Basic/DiagnosticIDs.cpp
@@ -30,9 +30,10 @@ namespace {
// Diagnostic classes.
enum {
CLASS_NOTE = 0x01,
- CLASS_WARNING = 0x02,
- CLASS_EXTENSION = 0x03,
- CLASS_ERROR = 0x04
+ CLASS_REMARK = 0x02,
+ CLASS_WARNING = 0x03,
+ CLASS_EXTENSION = 0x04,
+ CLASS_ERROR = 0x05
};
struct StaticDiagInfoRec {
@@ -409,6 +410,9 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, unsigned
DiagClass,
case diag::MAP_IGNORE:
Result = DiagnosticIDs::Ignored;
break;
+ case diag::MAP_REMARK:
+ Result = DiagnosticIDs::Remark;
+ break;
case diag::MAP_WARNING:
Result = DiagnosticIDs::Warning;
break;
@@ -425,6 +429,11 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID,
unsigned DiagClass,
!MappingInfo.isUser())
Result = DiagnosticIDs::Warning;
+ // Diagnostics of class REMARK are either printed as remarks or in case they
+ // have been added to -Werror they are printed as errors.
+ if (DiagClass == CLASS_REMARK && Result == DiagnosticIDs::Warning)
+ Result = DiagnosticIDs::Remark;
+
// Ignore -pedantic diagnostics inside __extension__ blocks.
// (The diagnostics controlled by -pedantic are the extension diagnostics
// that are not enabled by default.)
diff --git a/lib/CodeGen/CodeGenAction.cpp b/lib/CodeGen/CodeGenAction.cpp
index d7c03e8..2dab640 100644
--- a/lib/CodeGen/CodeGenAction.cpp
+++ b/lib/CodeGen/CodeGenAction.cpp
@@ -307,6 +307,27 @@ void BackendConsumer::InlineAsmDiagHandler2(const
llvm::SMDiagnostic &D,
case llvm::DS_Warning:
\
DiagID = diag::warn_fe_##GroupName;
\
break;
\
+ case llvm::DS_Remark:
\
+ llvm_unreachable("'remark' severity not expected");
\
+ break;
\
+ case llvm::DS_Note:
\
+ DiagID = diag::note_fe_##GroupName;
\
+ break;
\
+ }
\
+ } while (false)
+
+#define ComputeDiagRemarkID(Severity, GroupName, DiagID)
\
+ do {
\
+ switch (Severity) {
\
+ case llvm::DS_Error:
\
+ DiagID = diag::err_fe_##GroupName;
\
+ break;
\
+ case llvm::DS_Warning:
\
+ DiagID = diag::warn_fe_##GroupName;
\
+ break;
\
+ case llvm::DS_Remark:
\
+ DiagID = diag::remark_fe_##GroupName;
\
+ break;
\
case llvm::DS_Note:
\
DiagID = diag::note_fe_##GroupName;
\
break;
\
@@ -372,7 +393,7 @@ void BackendConsumer::DiagnosticHandlerImpl(const
DiagnosticInfo &DI) {
break;
default:
// Plugin IDs are not bound to any value as they are set dynamically.
- ComputeDiagID(Severity, backend_plugin, DiagID);
+ ComputeDiagRemarkID(Severity, backend_plugin, DiagID);
break;
}
std::string MsgStorage;
diff --git a/lib/Frontend/LogDiagnosticPrinter.cpp
b/lib/Frontend/LogDiagnosticPrinter.cpp
index 6e09a81..53b13fc 100644
--- a/lib/Frontend/LogDiagnosticPrinter.cpp
+++ b/lib/Frontend/LogDiagnosticPrinter.cpp
@@ -31,6 +31,7 @@ LogDiagnosticPrinter::~LogDiagnosticPrinter() {
static StringRef getLevelName(DiagnosticsEngine::Level Level) {
switch (Level) {
case DiagnosticsEngine::Ignored: return "ignored";
+ case DiagnosticsEngine::Remark: return "remark";
case DiagnosticsEngine::Note: return "note";
case DiagnosticsEngine::Warning: return "warning";
case DiagnosticsEngine::Error: return "error";
diff --git a/lib/Frontend/PrintPreprocessedOutput.cpp
b/lib/Frontend/PrintPreprocessedOutput.cpp
index 489227d..20d7cd3 100644
--- a/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -459,6 +459,9 @@ PragmaDiagnostic(SourceLocation Loc, StringRef Namespace,
MoveToLine(Loc);
OS << "#pragma " << Namespace << " diagnostic ";
switch (Map) {
+ case diag::MAP_REMARK:
+ OS << "remark";
+ break;
case diag::MAP_WARNING:
OS << "warning";
break;
diff --git a/lib/Frontend/TextDiagnostic.cpp b/lib/Frontend/TextDiagnostic.cpp
index 79f05c6..a0c3c2c 100644
--- a/lib/Frontend/TextDiagnostic.cpp
+++ b/lib/Frontend/TextDiagnostic.cpp
@@ -26,6 +26,8 @@ using namespace clang;
static const enum raw_ostream::Colors noteColor =
raw_ostream::BLACK;
+static const enum raw_ostream::Colors remarkColor =
+ raw_ostream::BLUE;
static const enum raw_ostream::Colors fixitColor =
raw_ostream::GREEN;
static const enum raw_ostream::Colors caretColor =
@@ -711,6 +713,7 @@ TextDiagnostic::printDiagnosticLevel(raw_ostream &OS,
case DiagnosticsEngine::Ignored:
llvm_unreachable("Invalid diagnostic type");
case DiagnosticsEngine::Note: OS.changeColor(noteColor, true); break;
+ case DiagnosticsEngine::Remark: OS.changeColor(remarkColor, true); break;
case DiagnosticsEngine::Warning: OS.changeColor(warningColor, true); break;
case DiagnosticsEngine::Error: OS.changeColor(errorColor, true); break;
case DiagnosticsEngine::Fatal: OS.changeColor(fatalColor, true); break;
@@ -721,6 +724,7 @@ TextDiagnostic::printDiagnosticLevel(raw_ostream &OS,
case DiagnosticsEngine::Ignored:
llvm_unreachable("Invalid diagnostic type");
case DiagnosticsEngine::Note: OS << "note"; break;
+ case DiagnosticsEngine::Remark: OS << "remark"; break;
case DiagnosticsEngine::Warning: OS << "warning"; break;
case DiagnosticsEngine::Error: OS << "error"; break;
case DiagnosticsEngine::Fatal: OS << "fatal error"; break;
diff --git a/tools/c-index-test/c-index-test.c
b/tools/c-index-test/c-index-test.c
index defc916..22c0d97 100644
--- a/tools/c-index-test/c-index-test.c
+++ b/tools/c-index-test/c-index-test.c
@@ -3736,6 +3736,7 @@ static const char *getDiagnosticCodeStr(enum
CXLoadDiag_Error error) {
static const char *getSeverityString(enum CXDiagnosticSeverity severity) {
switch (severity) {
case CXDiagnostic_Note: return "note";
+ case CXDiagnostic_Remark: return "remark";
case CXDiagnostic_Error: return "error";
case CXDiagnostic_Fatal: return "fatal";
case CXDiagnostic_Ignored: return "ignored";
diff --git a/tools/diagtool/ShowEnabledWarnings.cpp
b/tools/diagtool/ShowEnabledWarnings.cpp
index bcc7520..51213a4 100644
--- a/tools/diagtool/ShowEnabledWarnings.cpp
+++ b/tools/diagtool/ShowEnabledWarnings.cpp
@@ -44,6 +44,7 @@ static char getCharForLevel(DiagnosticsEngine::Level Level) {
switch (Level) {
case DiagnosticsEngine::Ignored: return ' ';
case DiagnosticsEngine::Note: return '-';
+ case DiagnosticsEngine::Remark: return 'R';
case DiagnosticsEngine::Warning: return 'W';
case DiagnosticsEngine::Error: return 'E';
case DiagnosticsEngine::Fatal: return 'F';
diff --git a/tools/libclang/CIndexDiagnostic.cpp
b/tools/libclang/CIndexDiagnostic.cpp
index d3450d5..a5440b4 100644
--- a/tools/libclang/CIndexDiagnostic.cpp
+++ b/tools/libclang/CIndexDiagnostic.cpp
@@ -305,6 +305,7 @@ CXString clang_formatDiagnostic(CXDiagnostic Diagnostic,
unsigned Options) {
switch (Severity) {
case CXDiagnostic_Ignored: llvm_unreachable("impossible");
case CXDiagnostic_Note: Out << "note: "; break;
+ case CXDiagnostic_Remark: Out << "remark: "; break;
case CXDiagnostic_Warning: Out << "warning: "; break;
case CXDiagnostic_Error: Out << "error: "; break;
case CXDiagnostic_Fatal: Out << "fatal error: "; break;
diff --git a/tools/libclang/CXStoredDiagnostic.cpp
b/tools/libclang/CXStoredDiagnostic.cpp
index 9731616..45ce39b 100644
--- a/tools/libclang/CXStoredDiagnostic.cpp
+++ b/tools/libclang/CXStoredDiagnostic.cpp
@@ -31,6 +31,7 @@ CXDiagnosticSeverity CXStoredDiagnostic::getSeverity() const {
switch (Diag.getLevel()) {
case DiagnosticsEngine::Ignored: return CXDiagnostic_Ignored;
case DiagnosticsEngine::Note: return CXDiagnostic_Note;
+ case DiagnosticsEngine::Remark: return CXDiagnostic_Remark;
case DiagnosticsEngine::Warning: return CXDiagnostic_Warning;
case DiagnosticsEngine::Error: return CXDiagnostic_Error;
case DiagnosticsEngine::Fatal: return CXDiagnostic_Fatal;
--
1.8.1.2
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
