[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:355-357
   }
+  else if (DisallowImplicitThisParam)
+    *DisallowImplicitThisParam = false;
----------------
jdenny wrote:
> aaron.ballman wrote:
> > Formatting is off here -- the `else if` should go up a line.
> I don't follow.  It looks right to me. 
Our coding standard is to write:
```
if (foo) {
} else if (bar) {
}
```
rather than
```
if (foo) {
}
else if (bar) {
}
```
One good way to catch this sort of thing is to run your patch through clang-format: https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting


================
Comment at: test/Sema/attr-print.cpp:1
+// RUN: %clang_cc1 %s -ast-print | FileCheck %s
+
----------------
jdenny wrote:
> aaron.ballman wrote:
> > -ast-print tests generally live in Misc rather than Sema -- can you move this test over to that directory?
> Sure.  Should the existing test/Sema/attr-print.c move too?
Ah, I didn't see we already had that one here. In that case, it's fine to leave this where it is.


https://reviews.llvm.org/D43248