<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Rafael,</p>
<p>i was looking at this proposal through the lens of tooling, like
clang-tidy, thats why the macros were interesting.</p>
<p>If the whole purpose is not to change the actual source-code but
more to change the code-generation macros are not so relevant, but
then the propsal should _NOT_ mix with current tooling that is
supposed to make changes in the source-code.</p>
<p>Not knowing a lot about the Codegeneration phases, but the AST
will be translated into IR at some point. They should be
equivalent in the program sense, but the AST might contain more
interesting information for your transformation as it's on a
higher level. In principle you could go to the IR.</p>
<p>Best Jonas<br>
</p>
<div class="moz-cite-prefix">Am 25.10.18 um 09:49 schrieb
Rafael·Stahl:<br>
</div>
<blockquote type="cite"
cite="mid:a592c0ba-3a82-0f67-482c-4d9cf1f1a541@tum.de">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<p>Hi Jonas</p>
<p>AST replacements are translated back into code by expanding
macros where required. This of course might lead to code
duplication and nasty inconsistency bugs when the programmer
then changes the macro. This is not an issue for my use-case
though: The transformations are applied _after_ programming,
it's more of a code generation step. Why not apply those on IR
level? For compatibility with other source-level tooling.</p>
<p>A great improvement for _during_ programming transformations
would be to limit macro expansion to the absolutely necessary
cases. Specifically: Don't expand when a change in the macro
argument would be equivalent and don't expand when a change in
the macro body would be equivalent. You could also do some
clever stuff like parameterizing the original macro with the
rewritten AST node, but that seems to intrusive to me.</p>
<p>@Manuel Yes, I will do that. Also to see if there is any
interest.<br>
</p>
<p>-Rafael</p>
<div class="moz-cite-prefix">On 24.10.18 10:30, Manuel Klimek
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAOsfVvmCAa7o2D=bm2MxkTdjSkiniseqGzm9_UY+EFAgiOF31Q@mail.gmail.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<div dir="ltr">If you want this in the core libs, a great idea
would be to write up a doc or email explaining the design and
the use cases.
<div>Note that if this doesn't solve a generic enough use
case, it might still be a good idea for it to live in a
different place - API surface is a cost to users.</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Tue, Oct 23, 2018 at 5:56 PM Jonas Toth via
cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org"
moz-do-not-send="true">cfe-dev@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0

.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<p>Hi Rafael,</p>
<p>nice to see your progress!</p>
<p>In my opinion these advancements should go in the
Tooling and Rewrite directly in clang but integrate with
the existing facilities. It would be inconvenient two
have two separate ways of doing rewritings.</p>
<p>What I am curious about is how to resolve the
macro-situation. If the refactoring will work directly
on the AST and ignore macros, how is the refactoring
translated into real code again?</p>
<p>Having an functional way of doing the refactorings with
just transforming the AST, e.g. replacing one node with
a new subtree would be great and elegant on the AST
side, but how would we move back from AST to code?</p>
<p>Best, Jonas<br>
</p>
<div class="m_-273721086631250889moz-cite-prefix">Am
23.10.18 um 17:32 schrieb Rafael·Stahl:<br>
</div>
<blockquote type="cite">
<p>Hey everyone</p>
<p>the two major limitations are resolved now:</p>
<p>- The macro argument issue<br>
- Support for replacing Decls using
clang::ast_type_traits::DynTypedNode<br>
</p>
<p>The macro issue vanished by itself once I figured a
little unintuitive detail out: The SourceLocations
from the original AST and the ones from the new
Preprocessor Lexer did not compare equal even though
they refer to the exact same location.</p>
<p>I believe this is because expansion SrcLocs have a
kind of pointer identity representation and equality
just compares raw representations, such that even when
they point to the same locations, they don't compare
equal. What worked for me was a kind of "deep"
comparison:<br>
</p>
<p>bool clutil::DeepSrcLocEqual(clang::SourceLocation
lhs, clang::SourceLocation rhs, const
clang::SourceManager &SM)<br>
{<br>
if (lhs == rhs)<br>
return true;<br>
<br>
if (SM.getExpansionLoc(lhs) !=
SM.getExpansionLoc(rhs))<br>
return false;<br>
if (SM.getSpellingLoc(lhs) !=
SM.getSpellingLoc(rhs))<br>
return false;<br>
<br>
clang::SourceLocation lhsMacro, rhsMacro;<br>
if (SM.isMacroArgExpansion(lhs, &lhsMacro))<br>
{<br>
if (!SM.isMacroArgExpansion(rhs,
&rhsMacro))<br>
return false;<br>
if (!DeepSrcLocEqual(lhsMacro, rhsMacro, SM))<br>
return false;<br>
}<br>
<br>
return true;<br>
}</p>
<p>Attached is the cleaned up rewriter that I ended up
with now. Still, if there is any interest, I'd be
happy to contribute this back to clangs libraries, but
I would require some feedback how this fits into
existing facilities.<br>
</p>
<p>Best regards<br>
Rafael<br>
</p>
<div class="m_-273721086631250889moz-cite-prefix">On
17.07.18 16:19, Jonas Toth wrote:<br>
</div>
<blockquote type="cite">
<p>Hi Rafael,</p>
<p>I did read into clang-refactor a while ago but
unfortunatly could not follow that up. If I recall
correctly its about source-to-source transformation
(as you said) and aims at implementing the primitive
refactorings that exist (e.g. extract-method,
extract-variable, ....).</p>
<p>Rewriting itself should happen with the normal
tooling framework.<br>
</p>
(<a class="m_-273721086631250889moz-txt-link-freetext"
href="https://clang.llvm.org/docs/RefactoringEngine.html"
target="_blank" moz-do-not-send="true">https://clang.llvm.org/docs/RefactoringEngine.html</a>)<br>
<br>
Maybe the implementers of the existing code can give
better comments on you proposal (and might have
considered a similar solution to yours already). <br>
<br>
+Alex Lorenz<br>
<br>
All the best, Jonas<br>
<br>
<br>
<div class="m_-273721086631250889moz-cite-prefix">Am
17.07.2018 um 14:46 schrieb Rafael·Stahl:<br>
</div>
<blockquote type="cite">
<p>Hi Jonas</p>
<p>Thanks for introducing me to this, I have seen
the "Replacement" before, but not clang-refactor.</p>
<p>However it seems to only provide management
facilities around rewrite operations and not aid
with the rewriting itself. Am I missing something
here?</p>
<p>The two core problems for me:</p>
<p>- nesting replacements: When implementing
replacements with clang-refactor, I still have to
provide replacements that are closed in
themselves. I cannot make them depend on others,
right?<br>
- macros: clang-refactor only seems to work with
spelling locations.</p>
<p>Maybe an even simpler example: Replace all
additions with "add(lhs, rhs)". This in itself is
very difficult with clang as soon as the Stmts are
nested or macros are involved.<br>
</p>
<p>Best regards<br>
Rafael<br>
</p>
<br>
<div class="m_-273721086631250889moz-cite-prefix">On
16.07.2018 19:06, Jonas Toth via cfe-dev wrote:<br>
</div>
<blockquote type="cite">
<p>Hi Rafael,</p>
<p>wouldn't your usecase be a task for
clang-refactor?</p>
<p>Best, Jonas<br>
</p>
<br>
<div class="m_-273721086631250889moz-cite-prefix">Am
16.07.2018 um 17:08 schrieb Rafael·Stahl via
cfe-dev:<br>
</div>
<blockquote type="cite">Hey everyone <br>
<br>
The rewriting API of Clang operates on the
source code in textual form. The user can use
AST nodes to figure out what to replace, but in
the end he has to remove and insert snippets in
a linear piece of text. <br>
<br>
This is very inconvenient when it is required to
restructure and nest replacements. The
involvement of macros makes a manual process
even more difficult. See some recent threads
expressing difficulty with the API [1][2]. <br>
<br>
What do I mean by "nested replacements"? For
example in the following: <br>
<br>
int i = x + s->a; <br>
<br>
I would want to replace the BinaryOperator with
a function call and the MemberExpr with some
constant: <br>
<br>
int i = Addition(x, 7); <br>
<br>
When keeping the two replacement rules
independent of each other, achieving this with
the current API is extremely difficult. More so
when macros are involved. <br>
<br>
I am proposing some kind of helper that aims to
solve these issues by providing an interface
that offers to directly replace AST nodes and a
mechanism to nest AST node replacements -
without having to worry about macros. <br>
<br>
Potential usage: <br>
<br>
- Develop a class that derives from
StmtToRewrite to define how replacements should
happen: <br>
<br>
class RewriteAdds : public cu::StmtToRewrite
<br>
{ <br>
public: <br>
std::string makeReplaceStr() const
override <br>
{ <br>
auto binOp =
dyn_cast<BinaryOperator>(replaceS); <br>
return "Addition(" +
getMgr()->getReplaced(binOp->getLHS()).strToInsert
+ ", " + <br>
getMgr()->getReplaced(binOp->getRHS()).strToInsert + ")"; <br>
} <br>
}; <br>
<br>
class RewriteMembs : public
cu::StmtToRewrite <br>
{ <br>
public: <br>
std::string makeReplaceStr() const
override <br>
{ <br>
return "7"; <br>
} <br>
}; <br>
<br>
- Construct a RewriteManager: <br>
<br>
cu::RewriteManager mgr(ACtx, PP); <br>
<br>
- Add rewriting operations to the manager: <br>
<br>
// std::vector<const Stmt *> AddStmts
= /* matched from binaryOperator() with plus */
<br>
// std::vector<const Stmt *> MembStmts
= /* matched from memberExpr() */ <br>
for (const auto &S : AddStmts)
mgr.registerStmt<RewriteAdds>(S); <br>
for (const auto &S : MembStmts)
mgr.registerStmt<RewriteMembs>(S); <br>
<br>
- Retrieve and apply the results: <br>
<br>
clang::Rewriter rewriter(SM, LangOpts); <br>
for (const auto &r :
mgr.getReplacements()) { <br>
rewriter.RemoveText(r.rangeToRemove); <br>
rewriter.InsertText(r.rangeToRemove.getBegin(),
r.strToInsert); <br>
} <br>
<br>
<br>
At the end of this mail is my low quality code
that kind-of implements this. TLDR: <br>
<br>
- Build a hierarchy of stmts to replace and keep
track of which replacements must be combined <br>
- Move further up in the AST if these
replacements are inside a macro <br>
- Recursively lex the file and look for
replacements outside-in by spelling locations.
Expand any macros that are encountered during
this. The re-lexing idea is based on the hint in
[3]. <br>
<br>
The code has the following shortcomings: <br>
<br>
- I do not know how to distinguish macro
argument expansions within macros. For example
in "#define FOO(a) a + a" the two "a"s expand to
different AST nodes that could be replaced with
different rules. This is an important issue,
because it can lead to completely broken code
with nesting. <br>
- Limited to Stmts, when Decls should be
supported too. <br>
- Very un-optimized with lexing the entire
source file many times. Easy to solve, but
didn't want to raise the complexity further for
now. <br>
- Could keep written code more clean by only
expanding macros if required. For example not
required if just a macro arg is replaced and all
expansions would be the same. <br>
<br>
<br>
I am very interested in your general thoughts.
I'm not very experienced with clang, but this
was my vision how I would want to do
replacements. Are you interested in getting this
into clang? I would need help with ironing out
the remaining issues. <br>
<br>
-Rafael <br>
<br>
<br>
[1] <a
class="m_-273721086631250889moz-txt-link-freetext"
href="http://lists.llvm.org/pipermail/cfe-dev/2018-July/058430.html"
target="_blank" moz-do-not-send="true">http://lists.llvm.org/pipermail/cfe-dev/2018-July/058430.html</a>
<br>
[2] <a
class="m_-273721086631250889moz-txt-link-freetext"
href="http://lists.llvm.org/pipermail/cfe-dev/2018-June/058213.html"
target="_blank" moz-do-not-send="true">http://lists.llvm.org/pipermail/cfe-dev/2018-June/058213.html</a>
<br>
[3] <a
class="m_-273721086631250889moz-txt-link-freetext"
href="http://lists.llvm.org/pipermail/cfe-dev/2017-August/055079.html"
target="_blank" moz-do-not-send="true">http://lists.llvm.org/pipermail/cfe-dev/2017-August/055079.html</a>
<br>
<br>
<br>
<br>
// snip<br>
<br>
<br>
<fieldset
class="m_-273721086631250889mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________
cfe-dev mailing list
<a class="m_-273721086631250889moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org" target="_blank" moz-do-not-send="true">cfe-dev@lists.llvm.org</a>
<a class="m_-273721086631250889moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<br>
<br>
<fieldset
class="m_-273721086631250889mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________
cfe-dev mailing list
<a class="m_-273721086631250889moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org" target="_blank" moz-do-not-send="true">cfe-dev@lists.llvm.org</a>
<a class="m_-273721086631250889moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
</div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank"
moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote>
</div>
</blockquote>
</blockquote>
</body>
</html>