[PATCH] Add diagnostic capabilities in LLVM (frontend part)

[Hal Finkel]

Hi Quentin,

Any reason you did not use phabricator for this patch?

+def BackendInlineAsm : DiagGroup<"backend-inline-asm">;

+def BackendStackSize : DiagGroup<"backend-stack-size">;

+def BackendPlugin : DiagGroup<"backend-plugin">;

Are these group names user-visible? If they are, do we want "backend" in the name?

+  // FIXME: We should demangle the function name.

+  // FIXME: Is there a way to get a location for that function?

+  FullSourceLoc Loc;

+  Diags.Report(Loc, diag::warn_fe_backend_stack_size)

+      << D.getStackSize() << D.getFunction().getName();

+  return true;

+}

+

We should figure this out ;) -- Furthermore, since you put in the work to have the diagnostic printers, we should use that to handle this stack-size warning instead of using this special class.

Looking through CodeGenModule.cpp, it does not look like there is such a map currently, although it looks like we could maintain such a map in the CodeGenModule::GetOrCreateLLVMFunction function.

 -Hal

----- Original Message -----
> From: "Quentin Colombet" <qcolombet at apple.com>
> To: "Dmitri Gribenko" <gribozavr at gmail.com>
> Cc: "llvm cfe" <cfe-commits at cs.uiuc.edu>
> Sent: Tuesday, December 17, 2013 5:03:36 PM
> Subject: Re: [PATCH] Add diagnostic capabilities in LLVM (frontend part)
> 
> 
> 
> Hi Dimitri,
> 
> Thanks for the quick reply!
> 
> New patch attached.
> 
> On Dec 17, 2013, at 11:58 AM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
> 
> > On Tue, Dec 17, 2013 at 11:22 AM, Quentin Colombet
> > <qcolombet at apple.com> wrote:
> >> - Tests.
> >> I made some tests locally, but the patch does not add any test
> >> case. The
> >> question is how can we add tests that require both a backend and a
> >> frontend
> >> in either the frontend or the backend?
> > 
> > A lot of tests in Clang have "REQUIRES: x86-registered-target" --
> > please take a look at them.
> Thanks, using your advice, I have added a test in
> test/Frontend/backend-diagnostic.c.
> 
> > 
> > About the patch:
> > 
> > 80 columns.
> Oh sorry about that, I was sure I had run clang-format… apparently
> not!
> Fixed.
> 
> > 
> > +/// DiagnosticHandler2 - This function is invoked when the backend
> > needs
> > +/// to report something to the user.
> > 
> > Please don't duplicate function name in comments.
> Fixed.
> 
> > 
> > +/// The \p DI interface provides two basic information:
> > +/// getKind() helps to determines what it is reporting and
> > +/// getSeverity() decribes how bad this is.
> > 
> > This does not really belong to the documentation for
> > DiagnosticHandler2(), I think.
> Removed.
> 
> > 
> > +#define SeveritySwitch(Severity, GroupName, DiagID)
> > \
> > + switch (Severity) {
> > \
> > 
> > Please wrap this in do {...} while(false), so that the trailing
> > semicolon in the macro invocation gets correctly attached to the
> > expansion.
> Done.
> 
> > 
> > Also, this macro could have a better name -- maybe ComputeSeverity?
> Renamed into ComputeDiagID.
> 
> Thanks,
> -Quentin
> 
> 
> 
> 
> > 
> > Dmitri
> > 
> > --
> > main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> > (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory