[PATCH] D35447: [Bash-autocompletion] Add support for -W<warning> and -Wno<warning>

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticIDs.h:266
 
+  static std::vector<std::string> getDiagnosticFlags();
+
----------------
Please write a function comment just like other member functions in this class.


================
Comment at: clang/lib/Basic/DiagnosticIDs.cpp:514
+std::vector<std::string> DiagnosticIDs::getDiagnosticFlags() {
+  std::vector<std::string> res;
+  for (int i = 1; DiagGroupNames[i] != '\0';) {
----------------
res -> Res
i -> I



================
Comment at: clang/lib/Basic/DiagnosticIDs.cpp:515
+  std::vector<std::string> res;
+  for (int i = 1; DiagGroupNames[i] != '\0';) {
+    std::string Diag =
----------------
`size_t` is more precise than `int`, even though sizeof(DiagGroupNames) in reality fits in an int.


================
Comment at: clang/lib/Basic/DiagnosticIDs.cpp:516-517
+  for (int i = 1; DiagGroupNames[i] != '\0';) {
+    std::string Diag =
+        std::string(StringRef(DiagGroupNames + i + 1, DiagGroupNames[i]));
+    i += DiagGroupNames[i] + 1;
----------------
If you want to construct a string, you can do this

  std::string Diag(DiagGroupNames + i + 1, DiagGroupNames[i]);


================
Comment at: clang/lib/Basic/DiagnosticIDs.cpp:519-522
+    std::string W = "-W" + Diag;
+    std::string Wno = "-Wno" + Diag;
+    res.push_back(W);
+    res.push_back(Wno);
----------------
I think you can remove these temporary variables:

  res.push_back("-W" + Diag);
  res.push_back("-Wno" + Diag);


================
Comment at: clang/lib/Driver/Driver.cpp:1279
+
+      for (std::string S : DiagnosticIDs::getDiagnosticFlags())
+        if (StringRef(S).startswith(PassedFlags))
----------------
Let's avoid copy: std::string -> StringRef



https://reviews.llvm.org/D35447