[PATCH] D47532: [ASTImporter] Import the whole redecl chain of functions

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu May 31 02:35:13 PDT 2018


martong added a comment.

I just wanted to give a detailed justification about why we should import the
whole redecl chain.  Consider the following code:

  void f(); // prototype
  void f() { f(); }

Currently, when we import the prototype we end up having two independent
functions with definitions:

  TranslationUnitDecl 0x25214c8 <<invalid sloc>> <invalid sloc>
  |
  |-FunctionDecl 0x255e3e8 <input.cc:1:11, col:27> col:16 f 'void ()'
  | `-CompoundStmt 0x255e4f0 <col:20, col:27>
  |   `-CallExpr 0x255e4c8 <col:22, col:24> 'void'
  |     `-ImplicitCastExpr 0x255e4b0 <col:22> 'void (*)()' <FunctionToPointerDecay>
  |       `-DeclRefExpr 0x255e488 <col:22> 'void ()' lvalue Function 0x255e3e8 'f' 'void ()'
  `-FunctionDecl 0x255e300 <col:1, col:27> col:6 f 'void ()'
    `-CompoundStmt 0x255e570 <col:20, col:27>
      `-CallExpr 0x255e548 <col:22, col:24> 'void'
        `-ImplicitCastExpr 0x255e530 <col:22> 'void (*)()' <FunctionToPointerDecay>
          `-DeclRefExpr 0x255e508 <col:22> 'void ()' lvalue Function 0x255e3e8 'f' 'void ()'

Also, when we import a definition of the friend function below we again end up
with two different definitions:

  struct X { friend void f(); };
  void f(){}

And what's more, one of these definitions has the class as a parent:

  `-CXXRecordDecl 0x1802358 <input0.cc:1:1, col:8> col:8 struct X definition
    |-CXXRecordDecl 0x1802478 <col:1, col:8> col:8 implicit struct X
    |-FunctionDecl 0x1802560 parent 0x17c5618 <col:12, col:40> col:24 f 'void ()'
    | `-CompoundStmt 0x1802610 <col:39, col:40>
    `-FriendDecl 0x1802620 <col:12, col:40> col:24
      `-FunctionDecl 0x1802560 parent 0x17c5618 <col:12, col:40> col:24 f 'void ()'
        `-CompoundStmt 0x1802610 <col:39, col:40>

In previous versions, we had similar problems in case of virtual functions,
i.e. the virtual flag of an out-of-class definition was not set, because we
missed to import the prototype which had the explicit `virtual` keyword. That
seems to be fixed even before this patch, but we wouldn't have that problem if
we had import the whole redecl chain.

These redundant definitions can cause assertions and can mislead any tool which
tries to process such a malformed AST.


Repository:
  rC Clang

https://reviews.llvm.org/D47532





More information about the cfe-commits mailing list