Hi,
in commit 207319 the remark libclang interface was removed by Alp due to
backward compatibility concerns.
The idea of this patch (and the identical original patch) is to expose
'remark' Diagnostics in the libclang interface. Is it implemented by
adding a new diagnostics type to the libclang diagnostic enum (obviously
not changing any existing enum values). This is the most straightforward
implementation.
The concerns raised in the revert commit where:
It trivially broke almost any stable application checking for
Severity >= CXDiagnostic_Error or indeed any other kind of severity
comparison upon encountering a 'remark'.
To not misunderstand this comment, if the user does not explicitly
enable 'remarks', libclang based applications are _not_ broken and
continue to work.
An issue may appear if both the user enables 'remark' diagnostics
explicitly and an application assumes that the set of diagnostic
types does never change and, based on this assumption, contains code
that checks the diagnostic type without handling unknown diagnostic
types gracefully.
This can be seen as backward compatibility issue, but my personal
interpretation is that the application is relying here on undocumented
assumptions. As possible issues only manifest in case the user
explicitly requests remarks, I believe the proposed patch is fine.
Alp, what is the actual use case that would break? Do you have an
application that relies on these assumptions? How likely is it that a
user will enable remark diagnostics for this application?
Any comments?
Tobias
P.S: Alp apparently has a better solution. I let him explain this
solution himself.
>From fef45afac59ce118fd41be3da9dcb0de86c20c16 Mon Sep 17 00:00:00 2001
From: Tobias Grosser <[email protected]>
Date: Mon, 28 Apr 2014 10:30:50 +0200
Subject: [PATCH] libclang: Add back 'CXDiagnostic_Remark'
This was removed in r207319 due to backward compatibility concerns from Alp.
---
include/clang-c/Index.h | 6 ++++++
tools/c-index-test/c-index-test.c | 1 +
tools/libclang/CIndexDiagnostic.cpp | 1 +
tools/libclang/CXLoadedDiagnostic.cpp | 3 +--
tools/libclang/CXStoredDiagnostic.cpp | 3 +--
5 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/clang-c/Index.h b/include/clang-c/Index.h
index 8393c6b..0174dac 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/tools/c-index-test/c-index-test.c b/tools/c-index-test/c-index-test.c
index ae2b81b..3796511 100644
--- a/tools/c-index-test/c-index-test.c
+++ b/tools/c-index-test/c-index-test.c
@@ -3754,6 +3754,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/libclang/CIndexDiagnostic.cpp b/tools/libclang/CIndexDiagnostic.cpp
index 8a57d7d..cf9dc6f 100644
--- a/tools/libclang/CIndexDiagnostic.cpp
+++ b/tools/libclang/CIndexDiagnostic.cpp
@@ -308,6 +308,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/CXLoadedDiagnostic.cpp b/tools/libclang/CXLoadedDiagnostic.cpp
index 8385f24..679c528 100644
--- a/tools/libclang/CXLoadedDiagnostic.cpp
+++ b/tools/libclang/CXLoadedDiagnostic.cpp
@@ -79,9 +79,8 @@ CXDiagnosticSeverity CXLoadedDiagnostic::getSeverity() const {
CASE(Warning)
CASE(Error)
CASE(Fatal)
+ CASE(Remark)
#undef CASE
- // The 'Remark' level isn't represented in the stable API.
- case serialized_diags::Remark: return CXDiagnostic_Warning;
}
llvm_unreachable("Invalid diagnostic level");
diff --git a/tools/libclang/CXStoredDiagnostic.cpp b/tools/libclang/CXStoredDiagnostic.cpp
index faaf746..45ce39b 100644
--- a/tools/libclang/CXStoredDiagnostic.cpp
+++ b/tools/libclang/CXStoredDiagnostic.cpp
@@ -31,8 +31,7 @@ CXDiagnosticSeverity CXStoredDiagnostic::getSeverity() const {
switch (Diag.getLevel()) {
case DiagnosticsEngine::Ignored: return CXDiagnostic_Ignored;
case DiagnosticsEngine::Note: return CXDiagnostic_Note;
- case DiagnosticsEngine::Remark:
- // The 'Remark' level isn't represented in the stable API.
+ 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.3.2
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
