<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 28/04/2014 12:59, Tobias Grosser
wrote:<br>
</div>
<blockquote cite="mid:535E429C.4090208@grosser.es" type="cite">On
28/04/2014 13:41, Tobias Grosser wrote:
<br>
<blockquote type="cite">On 28/04/2014 13:08, Alp Toker wrote:
<br>
<blockquote type="cite">
<br>
On 28/04/2014 11:11, Tobias Grosser wrote:
<br>
<blockquote type="cite">
<br>
<br>
On 28/04/2014 11:16, Alp Toker wrote:
<br>
<blockquote type="cite">CXDiagnostic with value 5 is higher
than CXDiagnostic_Error and there
<br>
are many applications using the outlined pattern that
break following
<br>
the change, either by crashing or mis-categorising
diagnostics as fatal
<br>
errors.
<br>
</blockquote>
<br>
I see that the above categorizes the diagnostic as an error.
This is
<br>
obviously incorrect. Nevertheless, it should just cause an
error
<br>
message. I do not see how/why this would crash.
<br>
</blockquote>
<br>
It hits an unreachable condition due to an unhandled case and
crashes.
<br>
</blockquote>
<br>
I consider this an application bug.
<br>
<br>
My quick look on ohloh did not find such usage. (Most results
are
<br>
duplicates of clang code, but e.g. ycm does the right thing and
just
<br>
maps to 'E' in case of unknown diagnostics). Obviously, there
may still
<br>
be other applications.
<br>
</blockquote>
<br>
I looked a little further. There are only seven projects on ohloh
which actually look at the diagnostic level (all others just have
a copy of clang somewhere). All seem to do something sensible
(either map to an unknown diagnostic string, map it as an error or
to a very high log level).
<br>
<br>
So the assumptions we are afraid of are to the very least not very
widespread on ohloh.
<br>
<br>
<br>
</blockquote>
<br>
Tobias, handling remarks as errors is not OK. It causes applications
that used to accept valid input to now bail out. That's a break that
can be triggered by users. At least a couple of these apps also seem
to enter an undefined state.<br>
<br>
I've emphasised your examples and added some more lines from the
context to point out the real impact:<br>
<br>
<br>
<br>
<blockquote cite="mid:535E429C.4090208@grosser.es" type="cite">##
YouCompleteMe
<br>
<br>
char DiagnosticSeverityToType( CXDiagnosticSeverity severity ) {
<br>
switch ( severity ) {
<br>
case CXDiagnostic_Ignored:
<br>
case CXDiagnostic_Note:
<br>
return 'I';
<br>
<br>
case CXDiagnostic_Warning:
<br>
return 'W';
<br>
<br>
case CXDiagnostic_Error:
<br>
case CXDiagnostic_Fatal:
<br>
return 'E';
<br>
</blockquote>
<br>
Handled as an error:<br>
<br>
<blockquote cite="mid:535E429C.4090208@grosser.es" type="cite">
<br>
<b> default:
</b><b><br>
</b><b> return 'E';
</b><br>
}
<br>
<br>
## go-clang
<br>
<br>
func (ds DiagnosticSeverity) String() string {
<br>
switch ds {
<br>
case Diagnostic_Ignored:
<br>
return "Ignored"
<br>
case Diagnostic_Note:
<br>
return "Note"
<br>
case Diagnostic_Warning:
<br>
return "Warning"
<br>
case Diagnostic_Error:
<br>
return "Error"
<br>
case Diagnostic_Fatal:
<br>
return "Fatal"
<br>
default:
<br>
return "Invalid"
<br>
}
<br>
}
<br>
</blockquote>
<br>
Handled as an error:<br>
<br>
<blockquote cite="mid:535E429C.4090208@grosser.es" type="cite">
<br>
## rust-bindgen
<br>
<br>
if d.severity() >= CXDiagnostic_Error {
<br>
c_err = true;
<br>
}
<br>
<br>
## bindgen.clay
<br>
<br>
if (clang_getDiagnosticSeverity(diag) >=
CXDiagnostic_Error)
<br>
error? = true;
<br>
<br>
</blockquote>
<br>
Treated as an error:<br>
<br>
<blockquote cite="mid:535E429C.4090208@grosser.es" type="cite">##
oovcde
<br>
<br>
<b> if(sev >= CXDiagnostic_Error)
</b><b><br>
</b><b> errType = ET_CompileErrors;
</b><br>
else
<br>
errType = ET_CompileWarnings;
<br>
<br>
}
<br>
<br>
## rtags
<br>
<br>
int logLevel = INT_MAX;
<br>
const CXDiagnosticSeverity severity =
clang_getDiagnosticSeverity(diagnostic);
<br>
switch (severity) {
<br>
case CXDiagnostic_Fatal:
<br>
case CXDiagnostic_Error:
<br>
logLevel = Error;
<br>
break;
<br>
case CXDiagnostic_Warning:
<br>
logLevel = Warning;
<br>
break;
<br>
case CXDiagnostic_Note:
<br>
logLevel = Debug;
<br>
break;
<br>
case CXDiagnostic_Ignored:
<br>
break;
<br>
}
<br>
</blockquote>
<meta charset="utf-8">
You omitted the next few lines which proceed with logLevel = INT_MAX
and treat the remarks as errors:<br>
<br>
}<br>
<br>
const CXSourceLocation diagLoc =
clang_getDiagnosticLocation(diagnostic);<br>
const Location loc = createLocation(diagLoc, 0);<br>
const uint32_t fileId = loc.fileId();<br>
if (mVisitedFiles.contains(fileId)) {<br>
<b> if (severity >= CXDiagnostic_Error)</b><b><br>
</b><b> ++mData->errors[fileId];</b><br>
<br>
<br>
Mapping remarks to warnings for now as I've done at least permits
these applications to function as the developer intended, which is
the highest priority for libclang users.<br>
<br>
Each traditional diagnostic severity has its own semantics:<br>
<br>
<b>Note</b>: Attached to another diagnostic.<br>
<b>Warning</b>: A message to the user.<br>
<b>Error</b>: A message about an issue that prevents compilation.<br>
<b>FatalError</b>: A message about an issue that has broken the
state of the frontend or diagnostic engine, such that serialization
may not be reliable.<br>
<br>
Your Remark severity has the same semantics as Warning, only with a
different display string.<br>
<br>
You still haven't answered why you chose to add a new diagnostic
level to clang, along with all the associated code churn. Given that
the semantics are identical this could have been trivially
implemented as a one-bit modifier on Warning, especially since the
only two uses of Remark<"%0"> are pass-throughs.<br>
<br>
I don't understand why you're pushing for something that's flawed
when there's potentially an elegant solution in sight and some
serious issues with the proposed one.<br>
<br>
Alp.<br>
<br>
<br>
<pre class="moz-signature" cols="72">--
<a class="moz-txt-link-freetext" href="http://www.nuanti.com">http://www.nuanti.com</a>
the browser experts
</pre>
</body>
</html>