<div dir="ltr"><div>Hi Aleksei,<br><br></div>Sorry for the late response.<br><div><div class="gmail_extra"><br><div class="gmail_quote">On 4 December 2017 at 16:56, Aleksei Sidorin <span dir="ltr"><<a href="mailto:a.sidorin@samsung.com" target="_blank">a.sidorin@samsung.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="m_-1818796837731716107moz-cite-prefix">Hello Gabor,<br>
<br>
This code will work, but it seems to be only a partial fix.<br></div></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div class="m_-1818796837731716107moz-cite-prefix">
You pointed the source of failure correctly. The whole problem is
bigger: the code responsible for importing definitions needs some
refactoring as well. I will prepare the patch after getting a test
case - it is not trivial now.<span class=""><br></span></div></div></blockquote><div><br></div><div>Yeah, unfortunately, this is really hard to reproduce without CTU. Looking forward to your patch. I agree that we do need some refactoring, this eager what we are doing right now is causing lots of troubles. This is not how the parser builds the AST, and I think the closer we can get in this regard, the fewer bugs will we have. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF"><div class="m_-1818796837731716107moz-cite-prefix"><span class="">
<br>
<br>
16.11.2017 19:42, Gábor Horváth via cfe-dev пишет:<br>
</span></div>
<blockquote type="cite">
<div dir="ltr">
<div>
<div>
<div>Hi Rafael!<br>
<br>
</div><div><div class="h5">
Yeah, the cross TU patch is a great way to detect issues
with the ASTImporter and unfortunately, those issues are
extremely hard to debug.<br>
<br>
</div></div></div><div><div class="h5">
Here, it looks like the source of the problem is that, when we
import an EnumConstantDecl, we will import the EnumDecl
without the typdef part.</div></div></div><div><div class="h5">
<div>A dirty quick fix for this particular case:</div>
<div><br>
</div>
<div>@@ -1782,6 +1807,11 @@ Decl
*ASTNodeImporter::<wbr>VisitEnumConstantDecl(<wbr>EnumConstantDecl *D) {<br>
DeclarationName Name;<br>
SourceLocation Loc;<br>
NamedDecl *ToD;<br>
+ auto *ED = cast<EnumDecl>(D-><wbr>getDeclContext());<br>
+ if (auto *TND = ED->getTypedefNameForAnonDecl(<wbr>)) {<br>
+ if (!Importer.Import(TND))<br>
+ return nullptr;<br>
+ }<br>
if (ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))<br>
return nullptr;<br>
if (ToD)<br>
<br>
</div>
<div>I did not have time yet to deep dive into this and look at
what the proper solution would be. Are you willing to continue
to investigate this?</div>
<div><br>
</div>
<div>Regards,</div>
<div>Gábor<br>
</div>
</div></div></div><div><div class="h5">
<div class="gmail_extra"><br>
<div class="gmail_quote">On 13 November 2017 at 12:16,
Rafael·Stahl via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Deal all<br>
<br>
While using the static analyzer with cross translation unit
support ( <a href="https://reviews.llvm.org/D30691" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3069<wbr>1</a>
), I stumbled upon an issue that is most likely related to
the ASTImporter.<br>
<br>
I'm using a custom checker, but this should be reproduced by
just running the scan-build tool.<br>
<br>
A simple reproducing example:<br>
<br>
main.c:<br>
<br>
------------------------------<wbr>---------<br>
void foo();<br>
void moo();<br>
<br>
int main()<br>
{<br>
foo();<br>
moo();<br>
}<br>
------------------------------<wbr>---------<br>
<br>
foo.c:<br>
<br>
------------------------------<wbr>---------<br>
#include "thing.h"<br>
<br>
void foo()<br>
{<br>
(void)THING_VALUE;<br>
}<br>
<br>
void conflict(thing_t type)<br>
{<br>
}<br>
------------------------------<wbr>---------<br>
<br>
moo.c:<br>
<br>
------------------------------<wbr>---------<br>
#include "thing.h"<br>
<br>
void moo()<br>
{<br>
conflict(THING_VALUE);<br>
}<br>
------------------------------<wbr>---------<br>
<br>
thing.h:<br>
<br>
------------------------------<wbr>---------<br>
typedef enum {<br>
THING_VALUE<br>
} thing_t;<br>
<br>
void conflict(thing_t type);<br>
------------------------------<wbr>---------<br>
<br>
Notes on particularities of this example:<br>
<br>
- main.c needs to NOT include the header<br>
- main() needs to call the functions in this order<br>
- foo() needs to reference the enumerator<br>
- the enum needs to be typedef'd<br>
<br>
If any of the above points do not apply, the issue does not
appear.<br>
<br>
The issue:<br>
<br>
------------------------------<wbr>---------<br>
In file included from moo.c:1:<br>
thing.h:1:9: warning: type 'thing_t' has incompatible<br>
definitions in different translation units [-Wodr]<br>
typedef enum {<br>
^<br>
thing.h:2:5: note: enumerator 'THING_VALUE' with value 0
here<br>
THING_VALUE<br>
^<br>
thing.h:1:9: note: no corresponding enumerator here<br>
foo.c:8:6: error: external function 'conflict' declared<br>
with incompatible types in different translation units
('void (thing_t)' vs. 'void (thing_t)')<br>
void conflict(thing_t type)<br>
^<br>
thing.h:5:6: note: declared here with type<br>
'void (thing_t)'<br>
void conflict(thing_t type);<br>
^<br>
------------------------------<wbr>---------<br>
<br>
After importing conflict(thing_t) the ASTImporter compared
the imported one with the original by structural
equivalence. This reveals that in the imported enum the
enumerators are missing, causing the above error.<br>
<br>
In my real code, not this example, I was unable to dump the
imported EnumDecl, because it eventually calls
SourceManager::isBeforeInTrans<wbr>lationUnit with two
SourceLocations from different files. Not sure if this is
related.<br>
<br>
I have tried adding:<br>
<br>
for (const auto Enumerator : D->enumerators())<br>
D2->addDecl(Importer.Import(En<wbr>umerator));<br>
<br>
before completing the definition in
ASTNodeImporter::VisitEnumDecl<wbr>(), but that results in:<br>
<br>
exe: tools/clang/lib/AST/DeclBase.c<wbr>pp:1374: void
clang::DeclContext::addHiddenD<wbr>ecl(clang::Decl*):
Assertion `!D->getNextDeclInContext() && D !=
LastDecl && "Decl already inserted into a
DeclContext"' failed.<br>
<br>
I'm not familiar enough with the ASTImporter to help myself
further here. Does anyone know what could be the issue?<br>
<br>
Best regards<span class="m_-1818796837731716107HOEnZb"><font color="#888888"><br>
Rafael Stahl<br>
<br>
<br>
</font></span><br>
______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
<br>
</blockquote>
</div>
<br>
</div>
<br>
<fieldset class="m_-1818796837731716107mimeAttachmentHeader"></fieldset>
<br>
<pre>______________________________<wbr>_________________
cfe-dev mailing list
<a class="m_-1818796837731716107moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>
<a class="m_-1818796837731716107moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a>
</pre>
</div></div></blockquote>
<p><br>
</p><span class="">
<pre class="m_-1818796837731716107moz-signature" cols="72">--
Best regards,
Aleksei Sidorin,
SRR, Samsung Electronics
</pre>
</span></div>
</blockquote></div><br></div></div></div>