[PATCH] D126832: [clang][tablegen] adds human documentation to `WarningOption`

Wed Jun 8 11:03:22 PDT 2022

cjdb added a comment.

In D126832#3567035 <https://reviews.llvm.org/D126832#3567035>, @aaron.ballman wrote:

> In D126832#3554816 <https://reviews.llvm.org/D126832#3554816>, @cjdb wrote:
>
>> In D126832#3553569 <https://reviews.llvm.org/D126832#3553569>, @aaron.ballman wrote:
>>
>>> Can you post some examples of the output from this option so we can see what the end results look like more easily?
>>
>> Right now this doesn't do anything at all, except feed the documentation from tablegen to Clang. It's just taking the text (represented by `...`) from `code Documentation [{...}]`, trimming any surrounding spaces, and putting that into `DIAG_ENTRY`.
>>
>> e.g.
>>
>>   code Documentation [{
>>     Hello, world!
>>   
>>     Goodbye, world!
>>   }]
>>
>> would be forwarded as `"Hello, world!\n\n  Goodbye, world!"`
>
> Thanks, that helps me to visualize what's going on. Any thoughts on how we could test this functionality, or are we going to assume that the testing comes from its usage when we actually make use of the new information threaded in?

Good question. Unless this functionality already has tests (and I'm gathering it doesn't), I think we can rely on Clang //being// the test. (Though we should consider ourselves on notice for adding robust unit tests at some point.)

================
Comment at: clang/utils/TableGen/ClangDiagnosticsEmitter.cpp:1550-1553
+    if (!Documentation.empty())
+      OS << "R\"(" << StringRef(Documentation).trim() << ")\"";
+    else
+      OS << R"("")";
----------------
aaron.ballman wrote:
> Isn't this functionally equivalent? (It means we're using a raw empty string rather than a normal empty string, but that shouldn't matter.)
Wow, I can no longer say I've never done `if (condition) return true; else return false;` 😂 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126832