r211380 - Fix crash caused by unnamed union or struct when doing ast-print

[Serge Pavlov]

As a possible way to actually test the generated code, the output, which
must be a valid C text, could be compiled again. The result of the second
compilation must be identical to the first one. For C this way indeed
works, the corresponding changes are:

diff --git a/test/Coverage/ast-printing.c b/test/Coverage/ast-printing.c
index ecaf3ab..eb22f92 100644
--- a/test/Coverage/ast-printing.c
+++ b/test/Coverage/ast-printing.c
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsyntax-only %s
-// RUN: %clang_cc1 -ast-print %s
+// RUN: %clang_cc1 -ast-print %s -o %t.1.c
+// RUN: %clang_cc1 -ast-print %t.1.c -o %t.2.c
+// RUN: diff %t.1.c %t.2.c
 // RUN: %clang_cc1 -ast-dump %s
 // RUN: %clang_cc1 -print-decl-contexts %s

However, for C++ and ObjC this way does not work - output in these cases is
not syntactically correct.

Do you think these changes may be checked in?

Thanks,
--Serge


2014-06-21 1:53 GMT+07:00 David Blaikie <dblaikie at gmail.com>:

> On Fri, Jun 20, 2014 at 10:08 AM, Serge Pavlov <sepavloff at gmail.com>
> wrote:
> > Author: sepavloff
> > Date: Fri Jun 20 12:08:28 2014
> > New Revision: 211380
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=211380&view=rev
> > Log:
> > Fix crash caused by unnamed union or struct when doing ast-print
> >
> > Modified:
> >     cfe/trunk/lib/AST/StmtPrinter.cpp
> >     cfe/trunk/test/Coverage/c-language-features.inc
> >
> > Modified: cfe/trunk/lib/AST/StmtPrinter.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtPrinter.cpp?rev=211380&r1=211379&r2=211380&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/lib/AST/StmtPrinter.cpp (original)
> > +++ cfe/trunk/lib/AST/StmtPrinter.cpp Fri Jun 20 12:08:28 2014
> > @@ -1274,10 +1274,12 @@ void StmtPrinter::VisitDesignatedInitExp
> >                        DEnd = Node->designators_end();
> >         D != DEnd; ++D) {
> >      if (D->isFieldDesignator()) {
> > -      if (D->getDotLoc().isInvalid())
> > -        OS << D->getFieldName()->getName() << ":";
> > -      else
> > +      if (D->getDotLoc().isInvalid()) {
> > +        if (IdentifierInfo *II = D->getFieldName())
> > +          OS << II->getName() << ":";
> > +      } else {
> >          OS << "." << D->getFieldName()->getName();
> > +      }
> >      } else {
> >        OS << "[";
> >        if (D->isArrayDesignator()) {
> >
> > Modified: cfe/trunk/test/Coverage/c-language-features.inc
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Coverage/c-language-features.inc?rev=211380&r1=211379&r2=211380&view=diff
> >
> ==============================================================================
> > --- cfe/trunk/test/Coverage/c-language-features.inc (original)
> > +++ cfe/trunk/test/Coverage/c-language-features.inc Fri Jun 20 12:08:28
> 2014
> > @@ -196,3 +196,15 @@ struct s11 {
> >    } f0;
> >    int f1;
> >  };
> > +
> > +// Unnamed structures.
> > +struct s12 {
> > +  struct {
> > +    unsigned char aa;
> > +    unsigned char bb;
> > +  };
> > +};
> > +
> > +void f11() {
> > +  struct s12 var = { .aa = 33 };
> > +}
>
> Any chance of actually testing the output of ast-dump to ensure it's
> correct for this code? "does not crash" isn't really a good test to
> write (& while this test file/case is already doing that, I would
> prefer not to perpetuate that sort of thing).
>
> - David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140622/a4631514/attachment.html>