thakis added inline comments.
Comment at: clang-tools-extra/trunk/clangd/refactor/tweaks/CMakeLists.txt:20
SwapIfBranches.cpp
+ ExtractVariable.cpp
(nit: keep in alphabetical order)
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
SureYeaah added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:187
+ std::string ExtractedVarDecl = std::string("auto ") + VarName.str() + " = " +
+ ExtractionCode.str() + "; ";
+ return
This revision was automatically updated to reflect the committed changes.
SureYeaah marked 8 inline comments as done.
Closed by commit rL365453: dummy variable extraction on a function scope
(authored by SureYeaah, committed by ).
Herald added a project: LLVM.
Herald added a subscriber:
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Awesome! Do you have commit access?
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:90
+bool isAFunctionRef(const clang::Expr *Expr) {
+
SureYeaah added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:90
+bool isAFunctionRef(const clang::Expr *Expr) {
+ const clang::DeclRefExpr *DeclRef = dyn_cast_or_null(Expr);
+ if (DeclRef && isa(DeclRef->getDecl()))
SureYeaah updated this revision to Diff 208174.
SureYeaah marked 21 inline comments as done.
SureYeaah added a comment.
Added whitelist for computeInsertionPoint
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63773/new/
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:42
+ // Generate Replacement for replacing selected expression with given VarName
+ tooling::Replacement replaceWithVar(std::string VarName) const;
+ // Generate
SureYeaah updated this revision to Diff 207522.
SureYeaah added a comment.
Removed check for braces and fixed code for finding insertionpoint
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63773/new/
https://reviews.llvm.org/D63773
Files:
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:71
+}
+std::vector Extract::getReferencedDecls() {
+ // RAV subclass to find all DeclRefs in a given Stmt
SureYeaah wrote:
> kadircet wrote:
> > this
SureYeaah marked 13 inline comments as done.
SureYeaah added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:71
+}
+std::vector Extract::getReferencedDecls() {
+ // RAV subclass to find all DeclRefs in a given Stmt
sammccall added a comment.
Comments re the `Extract` class.
It seems OK to me, i'm not really sure whether it's an improvement or not after
the pieces (needsBraces, checForStmt etc) are removed. Up to you.
(As the previous comments would reduce the scope of the patch, it'd be nice to
address
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:34
+// class to store information about the Expr that is being extracted
+class Extract {
+public:
most of the methods seem to be taking a SM, why not make
SureYeaah updated this revision to Diff 207084.
SureYeaah added a comment.
[Clangd] Refactored code
- Created new class Extract to store information about the expression being
extracted.
- Doesn't fix all of previous comments
Looking for comments on the new class stucture
Repository:
rG
SureYeaah marked 14 inline comments as done.
SureYeaah added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:173
+ // give up if extraction will take a variable out of scope
+ if (!extractionAllowed(ParStmt, N, M))
+
sammccall added a comment.
Please forgive the mix of high-level and low-level comments here.
Happy to discuss further of course!
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:55
+// checks whether any variable in a given expr is declared in the
SureYeaah updated this revision to Diff 206663.
SureYeaah marked 4 inline comments as done.
SureYeaah added a comment.
Refactored code
- Refactored code as pointed by kadircet
- Fixed crash for if statements without an else clause
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST
SureYeaah marked 12 inline comments as done.
SureYeaah added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:54
+
+// RAV subclass to find all DeclRefs in a given Stmt
+class FindDeclRefsVisitor
kadircet wrote:
> I
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp:54
+
+// RAV subclass to find all DeclRefs in a given Stmt
+class FindDeclRefsVisitor
I believe this class is rather used to check if any decl referenced in
SureYeaah created this revision.
SureYeaah added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, mgorny.
Herald added a project: clang.
SureYeaah retitled this revision from "dummy variable extraction on a function
scope" to "[clangd] dummy variable
19 matches
Mail list logo
