[PATCH] D104439: [analyzer][NFC] Demonstrate a move from the analyzer-configs `.def` file to a TableGen file

[Min-Yih Hsu via Phabricator via cfe-commits]

myhsu added a comment.

Just curious: Have you considered using llvm::opt or even llvm::cl infrastructure? What are the pros & cons?



================
Comment at: clang/utils/TableGen/ClangSAConfigsEmitter.cpp:24
+
+using SortedRecords = llvm::StringMap<const Record *>;
+
----------------



================
Comment at: clang/utils/TableGen/ClangSAConfigsEmitter.cpp:32
+  OS.write_escaped(R->getValueAsString("HelpText")) << "\",";
+  StringRef DefaultVal = R->getValueAsString("DefaultVal");
+
----------------
I'm not an expert in clang-analyzer, but can `DefaultVal` be optional in an analyzer config? It you want it to be optional -- which, I think, also makes the TableGen file more concise -- you can use `Record::getValueasOptionalString` instead.


================
Comment at: clang/utils/TableGen/ClangSAConfigsEmitter.cpp:57
+
+  OS << "// This file is automatically generated. Do not edit this file by "
+        "hand.\n";
----------------
Please use `llvm::emitSourceFileHeader` (in llvm/TableGen/TableGenBackend.h)


================
Comment at: clang/utils/TableGen/ClangSAConfigsEmitter.cpp:76
+    OS << "BOOLEAN_OPTION(bool,";
+    printOptionFields(R, OS, /*IsDefaultValString*/false);
+    OS << ")\n";
----------------
IIRC usually we will also write /*IsDefaultValString=*/


================
Comment at: clang/utils/TableGen/ClangSAConfigsEmitter.cpp:140
+    }
+    OS << ")\n";
+  }
----------------
if `R->isValueUnset("Values")` returns true, the resulting code will have a trailing comma just right before the right brace:
```
ENUM_OPTION(Foo, Foo, "bar", "...",)
```
which I don't think is valid for C/C++ macro.
Please use `llvm::ListSeparator`(in StringExtras.h) to print commas instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104439