[PATCH] D64717: [Clangd] Fixed ExtractVariable for MemberExprs and Assignment Exprs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:323
       while(a < ^1)
-        ^a++;
+        [[a++]];
       // do while 
----------------
SureYeaah wrote:
> kadircet wrote:
> > I thought extractor didn't handle this case(missing braces).
> > 
> > What is the extraction in this case? Because if it is
> > 
> > ```
> > auto dummy = a++;
> > while (a < 1)
> >    dummy++;
> > ```
> > 
> > it is not going to be semantically same.
> We don't check for missing braces and continuing traversing up the AST until we find the CompoundStmt. 
> 
> Yes, it's not going to be semantically the same. We only check if after extraction, the extracted expression will reference a variable that hasn't been declared till that point.
I believe this behavior is not expected and we should address that as well, the expressions you extract might have weird side effects, therefore if some expression is within a loop body it should stay there.
No need to address in this patch though, could you (leave a fixme || create a bug) to make sure extraction of such expressions doesn't go to outer scope, but rather stays within the loop body?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64717