Commit 7ae1b4a0 authored by Richard Smith's avatar Richard Smith
Browse files

Implement P1766R1: diagnose giving non-C-compatible classes a typedef name for linkage purposes.

Summary:
Due to a recent (but retroactive) C++ rule change, only sufficiently
C-compatible classes are permitted to be given a typedef name for
linkage purposes. Add an enabled-by-default warning for these cases, and
rephrase our existing error for the case where we encounter the typedef
name for linkage after we've already computed and used a wrong linkage
in terms of the new rule.

Reviewers: rjmccall

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74103
parent 884acbb9
Loading
Loading
Loading
Loading
+37 −0
Original line number Diff line number Diff line
@@ -103,6 +103,43 @@ C11 Feature Support
C++ Language Changes in Clang
-----------------------------

- Clang now implements a restriction on giving non-C-compatible anonymous
  structs a typedef name for linkage purposes, as described in C++ committee
  paper `P1766R1 <http://wg21.link/p1766r1>`. This paper was adopted by the
  C++ committee as a Defect Report resolution, so it is applied retroactively
  to all C++ standard versions. This affects code such as:

  .. code-block:: c++

    typedef struct {
      int f() { return 0; }
    } S;

  Previous versions of Clang rejected some constructs of this form
  (specifically, where the linkage of the type happened to be computed
  before the parser reached the typedef name); those cases are still rejected
  in Clang 11.  In addition, cases that previous versions of Clang did not
  reject now produce an extension warning. This warning can be disabled with
  the warning flag ``-Wno-non-c-typedef-for-linkage``.

  Affected code should be updated to provide a tag name for the anonymous
  struct:

  .. code-block:: c++

    struct S {
      int f() { return 0; }
    };

  If the code is shared with a C compilation (for example, if the parts that
  are not C-compatible are guarded with ``#ifdef __cplusplus``), the typedef
  declaration should be retained, but a tag name should still be provided:

  .. code-block:: c++

    typedef struct S {
      int f() { return 0; }
    } S;

C++1z Feature Support
^^^^^^^^^^^^^^^^^^^^^
+28 −6
Original line number Diff line number Diff line
@@ -788,10 +788,27 @@ def ext_no_declarators : ExtWarn<"declaration does not declare anything">,
def ext_typedef_without_a_name : ExtWarn<"typedef requires a name">,
  InGroup<MissingDeclarations>;
def err_typedef_not_identifier : Error<"typedef name must be an identifier">;
def err_typedef_changes_linkage : Error<"unsupported: typedef changes linkage"
  " of anonymous type, but linkage was already computed">;
def note_typedef_changes_linkage : Note<"use a tag name here to establish "
  "linkage prior to definition">;
def ext_non_c_like_anon_struct_in_typedef : ExtWarn<
  "anonymous non-C-compatible type given name for linkage purposes "
  "by %select{typedef|alias}0 declaration; "
  "add a tag name here">, InGroup<DiagGroup<"non-c-typedef-for-linkage">>;
def err_non_c_like_anon_struct_in_typedef : Error<
  "anonymous non-C-compatible type given name for linkage purposes "
  "by %select{typedef|alias}0 declaration after its linkage was computed; "
  "add a tag name here to establish linkage prior to definition">;
def err_typedef_changes_linkage : Error<
  "unsupported: anonymous type given name for linkage purposes "
  "by %select{typedef|alias}0 declaration after its linkage was computed; "
  "add a tag name here to establish linkage prior to definition">;
def note_non_c_like_anon_struct : Note<
  "type is not C-compatible due to this "
  "%select{base class|default member initializer|lambda expression|"
  "friend declaration|member declaration}0">;
def note_typedef_for_linkage_here : Note<
  "type is given name %0 for linkage purposes by this "
  "%select{typedef|alias}1 declaration">;
def err_statically_allocated_object : Error<
  "interface type cannot be statically allocated">;
def err_object_cannot_be_passed_returned_by_value : Error<
@@ -1796,8 +1813,13 @@ def note_nontrivial_objc_ownership : Note<
  "because type %0 has a member with %select{no|no|__strong|__weak|"
  "__autoreleasing}1 ownership">;
/// Selector for a TagTypeKind value.
def select_tag_type_kind : TextSubstitution<
  "%select{struct|interface|union|class|enum}0">;
def err_static_data_member_not_allowed_in_anon_struct : Error<
  "static data member %0 not allowed in anonymous struct">;
  "static data member %0 not allowed in anonymous "
  "%sub{select_tag_type_kind}1">;
def ext_static_data_member_in_union : ExtWarn<
  "static data member %0 in union is a C++11 extension">, InGroup<CXX11>;
def warn_cxx98_compat_static_data_member_in_union : Warning<
@@ -8118,7 +8140,7 @@ def err_reference_to_local_in_enclosing_context : Error<
  "%select{%3|block literal|lambda expression|context}2">;
def err_static_data_member_not_allowed_in_local_class : Error<
  "static data member %0 not allowed in local class %1">;
  "static data member %0 not allowed in local %sub{select_tag_type_kind}2 %1">;
// C++ derived classes
def err_base_clause_on_union : Error<"unions cannot have base classes">;
+141 −29
Original line number Diff line number Diff line
@@ -4346,6 +4346,84 @@ void Sema::handleTagNumbering(const TagDecl *Tag, Scope *TagScope) {
  }
}
namespace {
struct NonCLikeKind {
  enum {
    None,
    BaseClass,
    DefaultMemberInit,
    Lambda,
    Friend,
    OtherMember,
    Invalid,
  } Kind = None;
  SourceRange Range;
  explicit operator bool() { return Kind != None; }
};
}
/// Determine whether a class is C-like, according to the rules of C++
/// [dcl.typedef] for anonymous classes with typedef names for linkage.
static NonCLikeKind getNonCLikeKindForAnonymousStruct(const CXXRecordDecl *RD) {
  if (RD->isInvalidDecl())
    return {NonCLikeKind::Invalid, {}};
  // C++ [dcl.typedef]p9: [P1766R1]
  //   An unnamed class with a typedef name for linkage purposes shall not
  //
  //    -- have any base classes
  if (RD->getNumBases())
    return {NonCLikeKind::BaseClass,
            SourceRange(RD->bases_begin()->getBeginLoc(),
                        RD->bases_end()[-1].getEndLoc())};
  bool Invalid = false;
  for (Decl *D : RD->decls()) {
    // Don't complain about things we already diagnosed.
    if (D->isInvalidDecl()) {
      Invalid = true;
      continue;
    }
    //  -- have any [...] default member initializers
    if (auto *FD = dyn_cast<FieldDecl>(D)) {
      if (FD->hasInClassInitializer()) {
        auto *Init = FD->getInClassInitializer();
        return {NonCLikeKind::DefaultMemberInit,
                Init ? Init->getSourceRange() : D->getSourceRange()};
      }
      continue;
    }
    // FIXME: We don't allow friend declarations. This violates the wording of
    // P1766, but not the intent.
    if (isa<FriendDecl>(D))
      return {NonCLikeKind::Friend, D->getSourceRange()};
    //  -- declare any members other than non-static data members, member
    //     enumerations, or member classes,
    if (isa<StaticAssertDecl>(D) || isa<IndirectFieldDecl>(D) ||
        isa<EnumDecl>(D))
      continue;
    auto *MemberRD = dyn_cast<CXXRecordDecl>(D);
    if (!MemberRD)
      return {NonCLikeKind::OtherMember, D->getSourceRange()};
    //  -- contain a lambda-expression,
    if (MemberRD->isLambda())
      return {NonCLikeKind::Lambda, MemberRD->getSourceRange()};
    //  and all member classes shall also satisfy these requirements
    //  (recursively).
    if (MemberRD->isThisDeclarationADefinition()) {
      if (auto Kind = getNonCLikeKindForAnonymousStruct(MemberRD))
        return Kind;
    }
  }
  return {Invalid ? NonCLikeKind::Invalid : NonCLikeKind::None, {}};
}
void Sema::setTagNameForLinkagePurposes(TagDecl *TagFromDeclSpec,
                                        TypedefNameDecl *NewTD) {
  if (TagFromDeclSpec->isInvalidDecl())
@@ -4366,27 +4444,51 @@ void Sema::setTagNameForLinkagePurposes(TagDecl *TagFromDeclSpec,
    return;
  }
  // If we've already computed linkage for the anonymous tag, then
  // adding a typedef name for the anonymous decl can change that
  // linkage, which might be a serious problem.  Diagnose this as
  // unsupported and ignore the typedef name.  TODO: we should
  // pursue this as a language defect and establish a formal rule
  // for how to handle it.
  if (TagFromDeclSpec->hasLinkageBeenComputed()) {
    Diag(NewTD->getLocation(), diag::err_typedef_changes_linkage);
  // C++ [dcl.typedef]p9: [P1766R1, applied as DR]
  //   An unnamed class with a typedef name for linkage purposes shall [be
  //   C-like].
  //
  // FIXME: Also diagnose if we've already computed the linkage. That ideally
  // shouldn't happen, but there are constructs that the language rule doesn't
  // disallow for which we can't reasonably avoid computing linkage early.
  const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(TagFromDeclSpec);
  NonCLikeKind NonCLike = RD ? getNonCLikeKindForAnonymousStruct(RD)
                             : NonCLikeKind();
  bool ChangesLinkage = TagFromDeclSpec->hasLinkageBeenComputed();
  if (NonCLike || ChangesLinkage) {
    if (NonCLike.Kind == NonCLikeKind::Invalid)
      return;
    unsigned DiagID = diag::ext_non_c_like_anon_struct_in_typedef;
    if (ChangesLinkage) {
      // If the linkage changes, we can't accept this as an extension.
      if (NonCLike.Kind == NonCLikeKind::None)
        DiagID = diag::err_typedef_changes_linkage;
      else
        DiagID = diag::err_non_c_like_anon_struct_in_typedef;
    }
    SourceLocation FixitLoc =
        getLocForEndOfToken(TagFromDeclSpec->getInnerLocStart());
    llvm::SmallString<40> TextToInsert;
    TextToInsert += ' ';
    TextToInsert += NewTD->getIdentifier()->getName();
    SourceLocation tagLoc = TagFromDeclSpec->getInnerLocStart();
    tagLoc = getLocForEndOfToken(tagLoc);
    Diag(FixitLoc, DiagID)
      << isa<TypeAliasDecl>(NewTD)
      << FixItHint::CreateInsertion(FixitLoc, TextToInsert);
    if (NonCLike.Kind != NonCLikeKind::None) {
      Diag(NonCLike.Range.getBegin(), diag::note_non_c_like_anon_struct)
        << NonCLike.Kind - 1 << NonCLike.Range;
    }
    Diag(NewTD->getLocation(), diag::note_typedef_for_linkage_here)
      << NewTD << isa<TypeAliasDecl>(NewTD);
    llvm::SmallString<40> textToInsert;
    textToInsert += ' ';
    textToInsert += NewTD->getIdentifier()->getName();
    Diag(tagLoc, diag::note_typedef_changes_linkage)
        << FixItHint::CreateInsertion(tagLoc, textToInsert);
    if (ChangesLinkage)
      return;
  }
  // Otherwise, set this is the anon-decl typedef for the tag.
  // Otherwise, set this as the anon-decl typedef for the tag.
  TagFromDeclSpec->setTypedefNameForAnonDecl(NewTD);
}
@@ -4917,6 +5019,10 @@ Decl *Sema::BuildAnonymousStructOrUnion(Scope *S, DeclSpec &DS,
    //   define non-static data members. [Note: nested types and
    //   functions cannot be declared within an anonymous union. ]
    for (auto *Mem : Record->decls()) {
      // Ignore invalid declarations; we already diagnosed them.
      if (Mem->isInvalidDecl())
        continue;
      if (auto *FD = dyn_cast<FieldDecl>(Mem)) {
        // C++ [class.union]p3:
        //   An anonymous union shall not have private or protected
@@ -6757,28 +6863,33 @@ NamedDecl *Sema::ActOnVariableDeclarator(
    if (SC == SC_Static && CurContext->isRecord()) {
      if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(DC)) {
        if (RD->isLocalClass())
        // C++ [class.static.data]p2:
        //   A static data member shall not be a direct member of an unnamed
        //   or local class
        // FIXME: or of a (possibly indirectly) nested class thereof.
        if (RD->isLocalClass()) {
          Diag(D.getIdentifierLoc(),
               diag::err_static_data_member_not_allowed_in_local_class)
            << Name << RD->getDeclName();
            << Name << RD->getDeclName() << RD->getTagKind();
        } else if (!RD->getDeclName()) {
          Diag(D.getIdentifierLoc(),
               diag::err_static_data_member_not_allowed_in_anon_struct)
            << Name << RD->getTagKind();
          Invalid = true;
        } else if (RD->isUnion()) {
          // C++98 [class.union]p1: If a union contains a static data member,
          // the program is ill-formed. C++11 drops this restriction.
        if (RD->isUnion())
          Diag(D.getIdentifierLoc(),
               getLangOpts().CPlusPlus11
                 ? diag::warn_cxx98_compat_static_data_member_in_union
                 : diag::ext_static_data_member_in_union) << Name;
        // We conservatively disallow static data members in anonymous structs.
        else if (!RD->getDeclName())
          Diag(D.getIdentifierLoc(),
               diag::err_static_data_member_not_allowed_in_anon_struct)
            << Name << RD->isUnion();
        }
      }
    }
    // Match up the template parameter lists with the scope specifier, then
    // determine whether we have a template or a template specialization.
    bool InvalidScope = false;
    TemplateParams = MatchTemplateParametersToScopeSpecifier(
        D.getDeclSpec().getBeginLoc(), D.getIdentifierLoc(),
        D.getCXXScopeSpec(),
@@ -6786,7 +6897,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
            ? D.getName().TemplateId
            : nullptr,
        TemplateParamLists,
        /*never a friend*/ false, IsMemberSpecialization, Invalid);
        /*never a friend*/ false, IsMemberSpecialization, InvalidScope);
    Invalid |= InvalidScope;
    if (TemplateParams) {
      if (!TemplateParams->size() &&
+2 −2
Original line number Diff line number Diff line
@@ -2,9 +2,9 @@
// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=plist-multi-file -analyzer-config graph-trim-interval=5 -analyzer-config suppress-null-return-paths=false %s -o %t.plist
// RUN: %normalize_plist <%t.plist | diff -ub %S/Inputs/expected-plists/eager-reclamation-path-notes.cpp.plist -

typedef struct {
struct IntWrapper {
  int getValue();
} IntWrapper;
};

IntWrapper *getNullWrapper() {
  return 0;
+1 −1
Original line number Diff line number Diff line
@@ -165,7 +165,7 @@ class Holder1 { // no-warning
  TemplateSandwich<void *> t3;
};

typedef struct { // expected-warning{{Excessive padding in 'TypedefSandwich2'}}
typedef struct TypedefSandwich2 { // expected-warning{{Excessive padding in 'struct TypedefSandwich2'}}
  char c1;
  typedef struct { // expected-warning{{Excessive padding in 'TypedefSandwich2::NestedTypedef'}}
    char c1;
Loading