aaron.ballman added inline comments.
================
Comment at: clang-tidy/fuchsia/DefaultArgumentsCheck.cpp:35-38
+ SourceRange RemovalRange(
+ Lexer::getLocForEndOfToken(D->getLocation(), 0,
+ *Result.SourceManager, Result.Context->getLangOpts()),
+ D->getDefaultArgRange().getEnd()
----------------
juliehockett wrote:
> aaron.ballman wrote:
> > Does `getDefaultArgRange()` not provide the correct range already (so you
> > don't need to go back to the original source)? Or does that range miss the
> > `=`?
> >
> > You might want to disable the fix-it in the case `getDefaultArgRange()`
> > returns an empty range, or in case the default arg expression comes from a
> > macro (and add a test for the latter case). e.g.,
> > ```
> > #define DERP(val) = val
> >
> > void f(int i DERP);
> > ```
> > Additionally, a test where the identifier is elided would be good as well:
> > `void f(int = 12);`
> `getDefaultArgRange()` misses the `=` -- though if there's a better way to do
> it I'm all ears!
In that case, this seems fine to me.
================
Comment at: clang-tidy/fuchsia/DefaultArgumentsCheck.cpp:31
+ diag(S->getParam()->getLocStart(),
+ "the default parameter was declared here",
+ DiagnosticIDs::Note);
----------------
Drop "the" from the diagnostic to match wording in other places
================
Comment at: clang-tidy/fuchsia/DefaultArgumentsCheck.cpp:44
+ diag(DefaultArgRange.getBegin(),
+ "the default parameter was declared here",
+ DiagnosticIDs::Note);
----------------
Drop "the" here as well.
================
Comment at: clang-tidy/fuchsia/DefaultArgumentsCheck.cpp:48-52
+ if (!D->getName().empty()) {
+ StartLocation = D->getLocation();
+ } else {
+ StartLocation = D->getLocStart();
+ }
----------------
This can be hoisted to the initialization: `SourceLocation StartLocation =
D->getName().empty() ? D->getLocStart() : D->getLocation();`
================
Comment at: test/clang-tidy/fuchsia-default-arguments.cpp:40
+ return value;
+ // CHECK-MESSAGES: [[@LINE-2]]:12: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+ // CHECK-NEXT: note: the default parameter was declared here:
----------------
I think this diagnostic should be suppressed -- having it on the declaration
where the default argument is defined should be sufficient, no?
================
Comment at: test/clang-tidy/fuchsia-default-arguments.cpp:57
+// CHECK-MESSAGES: [[@LINE-1]]:8: warning: declaring a parameter with a
default argument is disallowed [fuchsia-default-arguments]
+// CHECK-FIXES-NOT: void h(int i);
----------------
Two more test cases that should be added:
```
void f(int i);
void f(int i = 12);
void f(int i) {
}
struct S {
void f(int i);
};
void S::f(int i = 12) {}
int main() {
S s;
s.f();
f();
}
```
https://reviews.llvm.org/D40108
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
