<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Some import helpers can make our life a
bit easier. Some examples below:<br>
<br>
template <typename DeclTy><br>
LLVM_NODISCARD bool importInto(DeclTy *&ToD, DeclTy
*FromD) {<br>
if (auto Res = Importer.Import(FromD)) {<br>
ToD = cast_or_null<DeclTy>(*Res);<br>
return false;<br>
}<br>
return true;<br>
}<br>
<br>
template <typename DeclTy><br>
LLVM_NODISCARD bool importIntoNonNull(DeclTy *&ToD, DeclTy
*FromD) {<br>
if (auto Res = Importer.Import(FromD)) {<br>
ToD = cast<DeclTy>(*Res);<br>
return false;<br>
}<br>
return true;<br>
}<br>
<br>
template <typename DeclTy> Optional<DeclTy *>
importCasted(DeclTy *FromD) {<br>
if (auto Res = Importer.Import(FromD))<br>
return cast_or_null<DeclTy>(*Res);<br>
return None;<br>
}<br>
<br>
template <typename DeclTy><br>
Optional<DeclTy *> importCastedNonNull(DeclTy *FromD) {<br>
if (auto Res = Importer.Import(FromD))<br>
return cast<DeclTy>(*Res);<br>
return None;<br>
}<br>
<br>
They can be used like this:<br>
<br>
QualType ASTNodeImporter::VisitTypedefType(const TypedefType *T) {<br>
if (auto ToDecl = importCastedNonNull(T->getDecl()))<br>
return Importer.getToContext().getTypeDeclType(*ToDecl);<br>
return {};<br>
}<br>
<br>
...<br>
<br>
<br>
if (importInto(ToEPI.ExceptionSpec.SourceDecl,<br>
FromEPI.ExceptionSpec.SourceDecl))<br>
return {};<br>
<br>
if (importInto(ToEPI.ExceptionSpec.SourceTemplate,<br>
FromEPI.ExceptionSpec.SourceTemplate))<br>
return {};<br>
<br>
...<br>
<br>
<br>
QualType ASTNodeImporter::VisitUnresolvedUsingType(<br>
const UnresolvedUsingType *T) {<br>
UnresolvedUsingTypenameDecl *ToD, *ToPrevD, *FromD =
T->getDecl();<br>
if (importInto(ToD, FromD) || importInto(ToPrevD,
FromD->getPreviousDecl()))<br>
return {};<br>
<br>
return Importer.getToContext().getTypeDeclType(ToD, ToPrevD);<br>
}<br>
<br>
Unfortunately, LLVM_NODISCARD is only enabled for C++14 which is
not our case.<br>
<br>
<br>
<br>
02.05.2018 01:28, George Karpenkov пишет:<br>
</div>
<blockquote type="cite"
cite="mid:EEA62F4F-E159-48DA-BC63-86F587D2AAB5@apple.com">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<br class="">
<div><br class="">
<blockquote type="cite" class="">
<div class="">On May 1, 2018, at 3:22 PM, Alexey Sidorin <<a
href="mailto:alexey.v.sidorin@ya.ru" class=""
moz-do-not-send="true">alexey.v.sidorin@ya.ru</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8" class="">
<div text="#000000" bgcolor="#FFFFFF" class="">
<div class="moz-cite-prefix">Hi George,<br class="">
<br class="">
You have wrote exactly the first idea I have described
in the initial proposal:<br class="">
> The problem with enforcing checks is the most
annoying one but also looks to be the simplest - we can
just replace the return value with Optional or ErrorOr
so clients will be enforced to perform a failure
checking.<br class="">
<br class="">
My question was about avoiding related boilerplate with
nested if's (or lots of early returns). Optionals are
excellent if one needs to do in-place checks. But if we
have >50 nested Import() calls (which is a pretty
common case for recursive ASTImporter), and we want to
check only the top-level result, their usage becomes
questionable. However, it still looks to be the best
option we have. Adding an argument passed by reference
in addition to Optional doesn't look good to me as well;
moreover, we have Expected/ErrorOr for these usage
scenarios.<br class="">
<br class="">
Anyway, maybe even my question itself is not correct.
The question of error handling doesn't exist on its own,
it is the result of ASTImporter design. Maybe a
potential solution is to split the lookup and structural
equivalence checking (to a kind of dry run) from node
import: in this case, Import() is called only after all
checks are done so it cannot fail. But this will need
ASTImporter redesign, and it is definitely not a thing
I'm going to work on soon.<br class="">
</div>
</div>
</div>
</blockquote>
<div><br class="">
</div>
Right, I see, sorry for misunderstanding. </div>
<div>The advantage of using “error” passed by reference is that it
could be reused across multiple `.import` calls;</div>
<div>having 50+ nested import calls could be mitigated by wrapping
related groups in functions,</div>
<div>that still seems to me a lesser evil than macros.</div>
<div><br class="">
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class="">
<div class="moz-cite-prefix"> <br class="">
01.05.2018 23:43, George Karpenkov via cfe-dev пишет:<br
class="">
</div>
<blockquote type="cite"
cite="mid:D671D24A-9A21-4B74-8601-3B7A8A55FE11@apple.com"
class="">
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8" class="">
<div class="">
<div class="">Hi Aleksei,</div>
<blockquote type="cite" class="">
<div class="">
<div style="word-wrap: break-word;
-webkit-nbsp-mode: space; line-break:
after-white-space;" class="">
<div class="">
<blockquote type="cite" class="">
<div class="">
<blockquote type="cite"
cite="mid:CAH6rKyCELnyVYeguSFfi=i_N+UyVMvJYaO=RnaZM2Mr3q_dCDQ@mail.gmail.com"
style="font-family: Helvetica;
font-size: 12px; font-style: normal;
font-variant-caps: normal; font-weight:
normal; letter-spacing: normal; orphans:
auto; text-align: start; text-indent:
0px; text-transform: none; white-space:
normal; widows: auto; word-spacing: 0px;
-webkit-text-size-adjust: auto;
-webkit-text-stroke-width: 0px;
background-color: rgb(255, 255, 255);
text-decoration: none;" class="">
<blockquote type="cite" class="">
<pre class="" wrap="">The problem of improving readability of these checks is a bit harder. The
most straightforward approach is to create a macro like:
#define IMPORT_PTR_INTO_VAR(VarName, Type, From) \
Type *VarName = nullptr; \
{ \
auto MacroImportRes = Importer.Import(From); \
if (!MacroImportRes) \
return MacroImportRes.getError(); \
VarName = *MacroImportRes; \
}
Same for non-pointer type. So, the final code will look like this:
ErrorOr<Expr *> ASTNodeImporter::VisitCXXMemberCallExpr(CXXMemberCallExpr
*E) {
IMPORT_QUAL_TYPE_INTO_VAR(T, E->getType())
IMPORT_PTR_INTO_VAR(ToFn, Expr, E->getCallee())
SmallVector<Expr *, 4> ToArgs(E->getNumArgs());
IMPORT_CONTAINER(E->arguments(), ToArgs);
return new (Importer.getToContext()) CXXMemberCallExpr(
Importer.getToContext(), ToFn, ToArgs, T, E->getValueKind(),
Importer.Import(E->getRParenLoc()));
}
But macros don't look clean. Unfortunately, I still haven't found a better
option so any suggestions are very welcome.
</pre>
</blockquote>
</blockquote>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
<div class="">
<div class=""><br class="">
</div>
<div class="">Macros are indeed ugly, and exceptions
are even worse.</div>
<div class="">I think a better solution would be to
return an Optional, and make `Importer.Import`
accept an out-parameter `Error`, passed by
reference.</div>
<div class="">Then you could write:</div>
<div class=""><br class="">
</div>
<div class="">Optional<Expr*>
VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {</div>
<div class=""> Error e;</div>
<div class=""> if (auto T =
Importer.import(E->getType(), e))</div>
<div class=""> if (auto ToFn =
Importer.import(E->getCallee(), e))</div>
<div class=""> if (auto ToArgs =
Importer.import(E->arguments(), e)</div>
<div class=""> return new
(Importer.getToContext()) CXXMemberCallExpr(…)</div>
<div class=""> outE = e;</div>
<div class=""> return none;</div>
<div class="">}</div>
<div class=""><br class="">
</div>
<div class="">That’s even more concise than your
example, explicit and does not need macros.</div>
<div class=""><br class="">
</div>
<div class="">This would be less verbose in C++17
where you would not need nested if’s,</div>
<div class="">but sadly we don’t have that in LLVM
yet.<span class="Apple-tab-span" style="white-space:pre"> </span></div>
<div class=""><br class="">
</div>
<div class="">George</div>
</div>
</div>
<br class="">
<fieldset class="mimeAttachmentHeader"></fieldset>
<br class="">
<pre class="" wrap="">_______________________________________________
cfe-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org" moz-do-not-send="true">cfe-dev@lists.llvm.org</a>
<a class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<p class=""><br class="">
</p>
</div>
</div>
</blockquote>
</div>
<br class="">
</blockquote>
<p><br>
</p>
</body>
</html>