[PATCH] D26582: [clang-tidy] Handle template instantiations in modenize-use-default check

[Malcolm Parsons via cfe-commits]

malcolm.parsons added inline comments.


================
Comment at: test/clang-tidy/modernize-use-default.cpp:142
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use '= default'
+// CHECK-FIXES: TempODef<T>::~TempODef() = default;
+
----------------
aaron.ballman wrote:
> malcolm.parsons wrote:
> > aaron.ballman wrote:
> > > Would it be better if the `= default` were on the inline declaration rather than the out-of-line definition? If so, that might be a reasonable FIXME to add to this test (assuming you don't feel like fixing it as part of this patch).
> > > 
> > > Also, does the `= delete` check suffer from the same issue?
> > There are valid reasons for having out-of-line definitions, even when defaulted.
> > The delete check only triggers if there is no definition. Template instantiation can't copy a definition that doesn't exist.
> Hmm, I suppose that is true. Thanks!
Scratch that.

The `= delete` check doesn't create conflicting duplicate fixes, but it does have an issue.
The fixes for template instantiations are correct for the operators, but not for the constructors.
This is because the AST has the wrong source range for the template instantiation's constructors.

The template's copy constructor's source range includes the parameters:
`CXXConstructorDecl 0x27bc9c0 <line:31:3, col:58> col:3 PositivePrivateTemplate<T> 'void (const PositivePrivateTemplate<T> &)'`

The template instantiation's copy constructor's source range does not:
`CXXConstructorDecl 0x27bd560 <line:31:3> col:3 PositivePrivateTemplate 'void (const struct PositivePrivateTemplate<int> &)'`

I'll make another patch for this.


https://reviews.llvm.org/D26582