[cfe-dev] [ASTImporter][CrossTU] inconsistent enumerators

Gábor Horváth via cfe-dev cfe-dev at lists.llvm.org
Mon Dec 4 08:25:50 PST 2017


Hi Aleksei,

Sorry for the late response.

On 4 December 2017 at 16:56, Aleksei Sidorin <a.sidorin at samsung.com> wrote:

> Hello Gabor,
>
> This code will work, but it seems to be only a partial fix.
>
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.
>

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.


>
>
> 16.11.2017 19:42, Gábor Horváth via cfe-dev пишет:
>
> Hi Rafael!
>
> Yeah, the cross TU patch is a great way to detect issues with the
> ASTImporter and unfortunately, those issues are extremely hard to debug.
>
> 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.
> A dirty quick fix for this particular case:
>
> @@ -1782,6 +1807,11 @@ Decl *ASTNodeImporter::VisitEnumConstantDecl(EnumConstantDecl
> *D) {
>    DeclarationName Name;
>    SourceLocation Loc;
>    NamedDecl *ToD;
> +  auto *ED = cast<EnumDecl>(D->getDeclContext());
> +  if (auto *TND = ED->getTypedefNameForAnonDecl()) {
> +    if (!Importer.Import(TND))
> +      return nullptr;
> +  }
>    if (ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
>      return nullptr;
>    if (ToD)
>
> 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?
>
> Regards,
> Gábor
>
> On 13 November 2017 at 12:16, Rafael·Stahl via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> Deal all
>>
>> While using the static analyzer with cross translation unit support (
>> https://reviews.llvm.org/D30691 ), I stumbled upon an issue that is most
>> likely related to the ASTImporter.
>>
>> I'm using a custom checker, but this should be reproduced by just running
>> the scan-build tool.
>>
>> A simple reproducing example:
>>
>> main.c:
>>
>> ---------------------------------------
>> void foo();
>> void moo();
>>
>> int main()
>> {
>>     foo();
>>     moo();
>> }
>> ---------------------------------------
>>
>> foo.c:
>>
>> ---------------------------------------
>> #include "thing.h"
>>
>> void foo()
>> {
>>     (void)THING_VALUE;
>> }
>>
>> void conflict(thing_t type)
>> {
>> }
>> ---------------------------------------
>>
>> moo.c:
>>
>> ---------------------------------------
>> #include "thing.h"
>>
>> void moo()
>> {
>>     conflict(THING_VALUE);
>> }
>> ---------------------------------------
>>
>> thing.h:
>>
>> ---------------------------------------
>> typedef enum {
>>     THING_VALUE
>> } thing_t;
>>
>> void conflict(thing_t type);
>> ---------------------------------------
>>
>> Notes on particularities of this example:
>>
>> - main.c needs to NOT include the header
>> - main() needs to call the functions in this order
>> - foo() needs to reference the enumerator
>> - the enum needs to be typedef'd
>>
>> If any of the above points do not apply, the issue does not appear.
>>
>> The issue:
>>
>> ---------------------------------------
>> In file included from moo.c:1:
>> thing.h:1:9: warning: type 'thing_t' has incompatible
>>       definitions in different translation units [-Wodr]
>> typedef enum {
>>         ^
>> thing.h:2:5: note: enumerator 'THING_VALUE' with value 0 here
>>     THING_VALUE
>>     ^
>> thing.h:1:9: note: no corresponding enumerator here
>> foo.c:8:6: error: external function 'conflict' declared
>>       with incompatible types in different translation units ('void
>> (thing_t)' vs. 'void (thing_t)')
>> void conflict(thing_t type)
>>      ^
>> thing.h:5:6: note: declared here with type
>>       'void (thing_t)'
>> void conflict(thing_t type);
>>      ^
>> ---------------------------------------
>>
>> 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.
>>
>> In my real code, not this example, I was unable to dump the imported
>> EnumDecl, because it eventually calls SourceManager::isBeforeInTranslationUnit
>> with two SourceLocations from different files. Not sure if this is related.
>>
>> I have tried adding:
>>
>> for (const auto Enumerator : D->enumerators())
>>     D2->addDecl(Importer.Import(Enumerator));
>>
>> before completing the definition in ASTNodeImporter::VisitEnumDecl(),
>> but that results in:
>>
>> exe: tools/clang/lib/AST/DeclBase.cpp:1374: void
>> clang::DeclContext::addHiddenDecl(clang::Decl*): Assertion
>> `!D->getNextDeclInContext() && D != LastDecl && "Decl already inserted into
>> a DeclContext"' failed.
>>
>> I'm not familiar enough with the ASTImporter to help myself further here.
>> Does anyone know what could be the issue?
>>
>> Best regards
>> Rafael Stahl
>>
>>
>>
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>
>
>
> _______________________________________________
> cfe-dev mailing listcfe-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
> --
> Best regards,
> Aleksei Sidorin,
> SRR, Samsung Electronics
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20171204/296c05c4/attachment-0001.html>


More information about the cfe-dev mailing list