[PATCH] Add warning capabilities in LLVM (backend part), Take 2

hfinkel at anl.gov hfinkel at anl.gov
Mon Dec 16 12:09:25 PST 2013



================
Comment at: include/llvm/Support/DiagnosticInfo.h:50
@@ +49,3 @@
+/// and assign it to each of its classes.
+/// The returned ID will be bigger than DK_FirstPluginKind. Thus, the plugin
+/// identifier are compatible with the statically known ones.
----------------
bigger than == "greater than or equal to"?

================
Comment at: include/llvm/Support/DiagnosticInfo.h:51
@@ +50,3 @@
+/// The returned ID will be bigger than DK_FirstPluginKind. Thus, the plugin
+/// identifier are compatible with the statically known ones.
+int getNextAvailablePluginDiagnosticKind();
----------------
I would say: Thus, the plugin identifiers will not conflict with the DiagnosticKind values.

================
Comment at: include/llvm/IR/LLVMContext.h:112
@@ +111,3 @@
+  /// (DI.Severity == RS_Error), so code should be prepared to drop the
+  /// erroneous construct on the floor and "not crash".
+  /// The generated code need not be correct.
----------------
How about: so the caller should leave the compilation process in a self-consistent state, even though the generated code need not be correct.

================
Comment at: include/llvm/Support/DiagnosticInfo.h:61
@@ +60,3 @@
+  /// Kind defines the kind of report this is about.
+  const DiagnosticKind Kind;
+  /// Severity gives the severity of the diagnostic.
----------------
As you pointed out to Tobi on the mailing list, this may hold values outside the range of valid enum values. As a result, this needs to be int.

================
Comment at: include/llvm/Support/DiagnosticInfo.h:71
@@ +70,3 @@
+
+  DiagnosticKind getKind() const { return Kind; }
+  DiagnosticSeverity getSeverity() const { return Severity; }
----------------
Likewise, this is also int. Just to be clear, you may want to write this as:
  /* DiagnosticKind */ int

================
Comment at: include/llvm/Support/DiagnosticInfo.h:66
@@ +65,3 @@
+public:
+  DiagnosticInfo(DiagnosticKind Kind, DiagnosticSeverity Severity)
+      : Kind(Kind), Severity(Severity) {}
----------------
int here too.

================
Comment at: lib/Support/DiagnosticInfo.cpp:31
@@ +30,3 @@
+  // make it thread safe.
+  return ++PluginKindID;
+}
----------------
We have an atomic increment in include/llvm/Support/Atomic.h -- I think using it is simpler than adding the FIXME ;)


http://llvm-reviews.chandlerc.com/D2376



More information about the llvm-commits mailing list