[cfe-commits] [review] Reimplementing variable definition semantics

[Douglas Gregor]

On Jan 28, 2010, at 2:20 PM, Sebastian Redl wrote:

> Hi,
> 
> Attached patch reimplements the "is a variable declaration a definition"
> code. This currently adds some stuff and replaces isTentativeDefinition.
> Follow-ups will then move more code over and eventually fix the C++
> redefinition issues. But I'd like this part of the code reviewed before
> I commit it. All test cases pass.
> 
> Sebastian
> Index: include/clang/AST/Decl.h
> ===================================================================
> --- include/clang/AST/Decl.h	(revision 94752)
> +++ include/clang/AST/Decl.h	(working copy)
> @@ -537,6 +537,7 @@
>   /// };
>   /// \endcode
>   bool isStaticDataMember() const {
> +    // If it wasn't static, it would be a FieldDecl.
>     return getDeclContext()->isRecord();
>   }
> 
> @@ -545,6 +546,26 @@
>     return const_cast<VarDecl*>(this)->getCanonicalDecl();
>   }
> 
> +  enum DefinitionKind {
> +    DeclarationOnly,      ///< This declaration is only a declaration.
> +    TentativeDefinition,  ///< This declaration is a tentative definition.
> +    Definition            ///< This declaration is definitely a definition.
> +  };
> +
> +  /// \brief Check whether this declaration is a definition. If this could be
> +  /// a tentative definition (in C), don't check whether there's an overriding
> +  /// definition.
> +  DefinitionKind isThisDeclarationADefinition() const;
> +
> +  /// \brief Determine whether this declaration acts as a definition. This is
> +  /// the case if it either is a definition, or the last tentative definition
> +  /// in the TU if there is no real definition.
> +  bool actsAsDefinition() const;
> +
> +  /// \brief Determine whether this is a tentative definition of a
> +  /// variable in C.
> +  bool isTentativeDefinitionNow() const;
> +
>   /// \brief Retrieve the definition of this variable, which may come
>   /// from a previous declaration. Def will be set to the VarDecl that
>   /// contains the initializer, and the result will be that
> @@ -578,10 +599,9 @@
>     return false;
>   }
> 
> -  /// \brief Determine whether this is a tentative definition of a
> -  /// variable in C.
> -  bool isTentativeDefinition(ASTContext &Context) const;
> -
> +  bool hasInit() const {
> +    return !Init.isNull();
> +  }
>   const Expr *getInit() const {
>     if (Init.isNull())
>       return 0;
> Index: lib/Sema/SemaDecl.cpp
> ===================================================================
> --- lib/Sema/SemaDecl.cpp	(revision 94752)
> +++ lib/Sema/SemaDecl.cpp	(working copy)
> @@ -3570,14 +3570,6 @@
>   // Attach the initializer to the decl.
>   VDecl->setInit(Context, Init);
> 
> -  // If the previous declaration of VDecl was a tentative definition,
> -  // remove it from the set of tentative definitions.
> -  if (VDecl->getPreviousDeclaration() &&
> -      VDecl->getPreviousDeclaration()->isTentativeDefinition(Context)) {
> -    bool Deleted = TentativeDefinitions.erase(VDecl->getDeclName());
> -    assert(Deleted && "Unrecorded tentative definition?"); Deleted=Deleted;
> -  }

If we're not erasing anything from the TentativeDefinitions set, then we probably don't need it at all; we'll just push all of the tentative definitions into the list and use !actsAsDefinition() to skip over the ones that won't become the real definition.

Everything else looks great. Thanks!

	- Doug