[PATCH] D65433: [clangd] DefineInline action availability checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp:183
+/// a.h:
+///   void foo() { return ; }
+///
----------------
hokein wrote:
> kadircet wrote:
> > hokein wrote:
> > > kadircet wrote:
> > > > hokein wrote:
> > > > > now we get a potential ODR violation in this example, maybe choose a different example?
> > > > You are right, but we cannot get away from it by changing the example. Adding an "inline " instead, and I believe that's what we should do if we are moving a function definition to a header.
> > > I think not all cases will need inline, e.g. class method, or function template(?).  Fix these problems is out-scope of the tweak (there is a clang-tidy check handling this case), and probably add complexity to the implementation. I'm leaning on not fixing it.
> > > 
> > > Maybe a better example is class method?
> > yes clang-tidy can generate those fixes but I believe a code-action should not generate code that relies on a clang-tidy check to fix it.
> I'm still not convinced that we should fix it in the check. but we don't have to solve it out in this patch, could we remove the inline from the sample? (adding it now seems to confuse users, we don't support it yet).
we basically support nothing in that sample in this patch, it is just to reflect the end-state of the code-action.
but as you wish; deleting it now, and instead adding with the next patch(that implements apply).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65433/new/

https://reviews.llvm.org/D65433