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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
