[PATCH] D146654: [clang] replaces numeric SARIF ids with heirarchical names

[Denis Nikitin via Phabricator via cfe-commits]

denik added inline comments.


================
Comment at: clang/lib/Basic/DiagnosticIDs.cpp:103
+// Stable names
+const char *const StaticDiagInfoStableNames[] = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR,     \
----------------
As of now it's 4600 strings, around 150k characters. Doesn't look bad but I think it should be mentioned in the change message because it affects all diagnostic.


================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:51-52
+      Diag->getDiags()->getDiagnosticIDs()->getStableName(Diag->getID()).str();
+  std::replace(StableName.begin(), StableName.end(), '_', '.');
+  SarifRule Rule = SarifRule::create().setRuleId(StableName);
 
----------------
§3.5.4 says that the hierarchical strings are separated by "/".

But more generally, are diagnostic names really fall under "hierarchical" definition?
Words between underscores should be treated as components. And $3.27.5 says that the leading components have to be the identifier of the rule.
In some cases they look like valid components, e.g. `err_access`, `err_access_dtor`, `err_access_dtor_exception`.
But in cases like `err_cannot_open_file` neither of the leading components exists.

Theoretically we could use groups as the leading component for warnings for example. For errors the leading components are probably not even necessary, since if I understood correctly they are needed to suppress subsets of violations on the SARIF consumer side.
Or we just could keep the names with underscores as is. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146654/new/

https://reviews.llvm.org/D146654