erichkeane added inline comments.
================
Comment at: clang/lib/AST/DeclPrinter.cpp:270
+
+ // FIXME: Find a way to use the AttrList.inc. We use if-else statements
+ // to classify each of them.
----------------
I think this is something we need to just do the right way, right away. The
below is completely unsustainable, and is just going to encourage us to spend
the next few years messing with this if/else-if tree. I'll leave final
judgement to Aaron, but just making this its own function dependent on
TableGen'ed files seems like what we should be doing from the start.
================
Comment at: clang/test/Analysis/blocks.mm:81
// ANALYZER-NEXT: 2: [B1.1] (CXXConstructExpr, [B1.3],
StructWithCopyConstructor)
-// CHECK-NEXT: 3: StructWithCopyConstructor s(5)
__attribute__((blocks("byref")));
+// CHECK-NEXT: 3: StructWithCopyConstructor s
__attribute__((blocks("byref")))(5);
// CHECK-NEXT: 4: ^{ }
----------------
aaron.ballman wrote:
> giulianobelinassi wrote:
> > aaron.ballman wrote:
> > > I can't quite tell if this change is good, bad, or just different.
> > This indeed doesn't look good, but for what it is worth it is still
> > corretly accepted by clang and gcc.
> I think this is a regression in terms of readability, but perhaps it's one we
> can live with.
So I have a BIG concern here. The primary purpose of AST print is to be
readable. I don't think we should be willing to compromise that for what is,
to the project, a non-goal.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141714/new/
https://reviews.llvm.org/D141714
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
