<div dir="ltr">  Hi Andrzej,<div><br></div><div>success!</div><div><br></div><div>The trick was that I needed to add the setForceEmit() call and only that. But I'd already got sidetracked with rolling my own DiagConsumer. When I removed all that it worked perfectly (but I did have to leave in the setForceEmit() - not sure why - maybe my FEAction or ASTConsumer classes are not doing some finalization calls that. I'll have to look closely at the CSC example again. </div><div><br></div><div>Thanks for your help and clang-tutor. I will undoubtedly be referring to clang-tutor again in the future.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 15 Oct 2020 at 09:51, Andrzej Warzynski via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Billy,<br>
<br>
Are you more interested in:<br>
   * understanding the Diagnostics API within Clang? or<br>
   * writing a tool that can emit diagnostics?<br>
<br>
The former will be much easier and straightforward if you use the <br>
clang::tooling API. Please, see example here:<br>
* <br>
<a href="https://github.com/banach-space/clang-tutor/blob/master/tools/CodeStyleCheckerMain.cpp#L45-L47" rel="noreferrer" target="_blank">https://github.com/banach-space/clang-tutor/blob/master/tools/CodeStyleCheckerMain.cpp#L45-L47</a><br>
With clang::tooling, DiagnosticsEngine and DiagnosticConsumer are set <br>
and managed for you behind the scenes (i.e. you shouldn't need to do <br>
anything extra). If you check a backtrace in your debugger, you'll <br>
notice that the driver within libclangFrontend manages that for you <br>
(i.e. creates a CompilerInstance, which contains DiagnosticEngine, which <br>
contains the default DiagnosticConsumer -> more or less ;-) ).<br>
<br>
However, if you _are_ interested in the diagnostics APIs, then like Ray <br>
has highlighted - managing the resources is often the root cause of most <br>
issues. These APIs are quite complex though and this can take a fair bit <br>
of trial and error. Having said that, do upload your code on GitHub and <br>
I can take a look if you want.<br>
<br>
-Andrzej<br>
<br>
On 14/10/2020 20:33, Ray Zhang via cfe-dev wrote:<br>
> Hi Billy,<br>
> <br>
> I've also run into this issue but I skirted around it by creating a <br>
> FixItRewriter <br>
> <<a href="https://clang.llvm.org/doxygen/classclang_1_1FixItRewriter.html" rel="noreferrer" target="_blank">https://clang.llvm.org/doxygen/classclang_1_1FixItRewriter.html</a>> which <br>
> is actually a DiagnosticConsumer which the diagnostics engine DOESN'T <br>
> own. Unsure if this is the same case that you're running into, but when <br>
> I was experiencing segfaults I found it to be an issue with lifetime <br>
> management for diagnostic consumers. YMMV since the issue is within the <br>
> FixItRewriter impl itself, but the class stores a pointer to the current <br>
> diagnostic engine's client before setting the instance of FixItRewriter <br>
> as our own. If we tell the engine to own the FixItRewriter, then the <br>
> previous consumer is destroyed, and our FixItRewriter is then pointing <br>
> to a destroyed resource. Can you try creating a std::unique_ptr for your <br>
> own diagnostics consumer and tell the engine not to own it? You'd have <br>
> to take care of the storage lifetime in this case.<br>
> <br>
> Also, one more thing is, when the Builder object gets destroyed it <br>
> performs an Emit(), and if you previously Emit then it won't do a <br>
> double-write. At the closing curly brace of your VisitFunctionDecl, it <br>
> may be a good idea to check which consumer you are currently using. I <br>
> did a GDB backtrace in my situation at the end-curly-brace and found <br>
> that the diagnostics consumer used to emit the message was actually not <br>
> the one I thought I was using.<br>
> <br>
> Since I didn't use the Visitor pattern but rather the Matcher pattern, <br>
> my scope of how the two use cases differ are limited. Hope you find the bug!<br>
> <br>
> Best,<br>
> Ray<br>
> <br>
> On Wed, Oct 14, 2020 at 12:17 PM Billy O'Mahony via cfe-dev <br>
> <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a> <mailto:<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>>> wrote:<br>
> <br>
>     Hi,<br>
> <br>
>     After much hacking I managed to write a DiagnosticConsumer that<br>
>     would emit some error messages for me. Kudos to Andrzej for his help.<br>
> <br>
>     I was expecting that I would more or less automatically get<br>
>     filename, line number and some source location carets added and some<br>
>     nice coloured text on my console. So for that I think I might need<br>
>     to use TextDiagnosticPrinter? However when I try to setClient that<br>
>     in my ASTContext's diagEngine it segfaults when a diagnostic is emitted.<br>
> <br>
>     I've tried looking in the clang/tools files for inspiration from<br>
>     other FrontendAction style tools but they either don't use<br>
>     TextDiagnosticPrinter or else they use some other clang infra<br>
>     like clang/Rewrite/Core/Rewriter.h.<br>
> <br>
>     This really feels like it should be super simple but I'm finding it<br>
>     very frustrating. My tool is actually doing useful things to spot<br>
>     project-specific code defects but now adding something simple like<br>
>     neat error messages is turning into a total quagmire.<br>
> <br>
>     Thanks,<br>
>     Billy.<br>
> <br>
>     class MyDiagnosticConsumer : public clang::DiagnosticConsumer {<br>
>     public:<br>
>          void HandleDiagnostic(clang::DiagnosticsEngine::Level<br>
>     DiagLevel, const clang::Diagnostic& Info) override {<br>
>              llvm::SmallVector<char, 512> message;<br>
>              Info.FormatDiagnostic(message);<br>
>              llvm::errs() << message << '\n';<br>
>               cout << "Hello HandleDiagnostic!" << endl;<br>
>          }<br>
>     };<br>
> <br>
>     class MyVisitor : public RecursiveASTVisitor<MyVisitor> {<br>
>     public:<br>
>        explicit MyVisitor(ASTContext *context)<br>
>          : mContext(context) {}<br>
> <br>
>        bool VisitFunctionDecl(FunctionDecl* fnDecl) {<br>
>            // let's just issue an error on every function decl!<br>
> <br>
>            auto& diagEngine = mContext->getDiagnostics();<br>
>            const auto ID =<br>
>     diagEngine.getCustomDiagID(clang::DiagnosticsEngine::Error,<br>
>                                                       "%0 declared? You<br>
>     insensitive clod!");<br>
>            auto Builder = diagEngine.Report(fnDecl->getLocation(), ID);<br>
>            Builder.AddString(fnDecl->getNameAsString());<br>
>           <br>
>     Builder.AddSourceRange(clang::CharSourceRange::getCharRange(call->getSourceRange()));<br>
>            Builder.setForceEmit();  // <<< without this<br>
>     MyDiagnosticConsumer::HandleDiagnostic is never called !!<br>
> <br>
>            return true;<br>
>        }<br>
> <br>
>     private:<br>
>        ASTContext *mContext;<br>
>     };<br>
> <br>
> <br>
>     class MyConsumer : public clang::ASTConsumer {<br>
>     public:<br>
>        explicit MyConsumer(ASTContext *Context) : Visitor(Context) {<br>
> <br>
>            DiagnosticsEngine &diagEngine = Context->getDiagnostics();<br>
> <br>
>            // if I set my own DiagnosticsConsumer here it works (but no<br>
>     line/file info).<br>
>            diagEngine.setClient(new MyDiagnosticConsumer(),<br>
>     /*ShouldOwnClient=*/true);<br>
> <br>
>            ---- OR ----<br>
> <br>
>            // if I set a TextDiagnosticPrinter it crashes later on<br>
>            // At the point where I otherwise get an error like<br>
>     '.../include/bla.h' file not found.<br>
>            IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new<br>
>     DiagnosticOptions();<br>
>            TextDiagnosticPrinter *DiagClient =<br>
>                new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);<br>
>            diagEngine.setClient(DiagClient, true); // true =><br>
>     shouldOwnClient<br>
>        }<br>
> <br>
>        virtual void HandleTranslationUnit(clang::ASTContext &Context) {<br>
>            auto Decls = Context.getTranslationUnitDecl()->decls();<br>
>            auto &SM = Context.getSourceManager();<br>
>            for (auto &Decl : Decls) {<br>
>                const auto& FileID = SM.getFileID(Decl->getLocation());<br>
>                if (FileID != SM.getMainFileID()) {<br>
>                    // Skip decls coming via #incl<br>
>                    continue;<br>
>                }<br>
> <br>
>                Visitor.TraverseDecl(Decl);<br>
>            }<br>
>        }<br>
> <br>
>     private:<br>
>        MyVisitor Visitor;<br>
>     };<br>
> <br>
> <br>
> <br>
> <br>
>     Program received signal SIGSEGV, Segmentation fault.<br>
>     clang::DiagnosticRenderer::emitDiagnostic (this=0x0, Loc=...,<br>
>     Level=clang::DiagnosticsEngine::Fatal, Message=..., Ranges=...,<br>
>          FixItHints=..., D=...) at<br>
>     /home/bomahony/llvm/tools/clang/lib/Frontend/DiagnosticRenderer.cpp:95<br>
>     95        beginDiagnostic(D, Level);<br>
>     (gdb) bt<br>
>     #0  clang::DiagnosticRenderer::emitDiagnostic (this=0x0, Loc=...,<br>
>     Level=clang::DiagnosticsEngine::Fatal, Message=..., Ranges=...,<br>
>          FixItHints=..., D=...) at<br>
>     /home/bomahony/llvm/tools/clang/lib/Frontend/DiagnosticRenderer.cpp:95<br>
>     #1  0x00000000084ce036 in<br>
>     clang::TextDiagnosticPrinter::HandleDiagnostic (this=0x9b22430,<br>
>     Level=clang::DiagnosticsEngine::Fatal, Info=...)<br>
>          at<br>
>     /home/bomahony/llvm/tools/clang/lib/Frontend/TextDiagnosticPrinter.cpp:152<br>
> <br>
> <br>
>     _______________________________________________<br>
>     cfe-dev mailing list<br>
>     <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a> <mailto:<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>><br>
>     <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
> <br>
> <br>
> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
> <br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>