Skip to content

Commit

Permalink
[C++20] [Modules] Remove previous workaround for odr hashing enums
Browse files Browse the repository at this point in the history
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
  • Loading branch information
ChuanqiXu9 authored and tstellar committed Feb 14, 2024
1 parent 6b9111d commit 729a0b6
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 51 deletions.
3 changes: 0 additions & 3 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1041,9 +1041,6 @@ Bug Fixes to C++ Support
in different visibility.
Fixes (`#67893 <https://github.com/llvm/llvm-project/issues/67893>`_)

- Fix a false-positive ODR violation for different definitions for `std::align_val_t`.
Fixes (`#76638 <https://github.com/llvm/llvm-project/issues/76638>`_)

- Remove recorded `#pragma once` state for headers included in named modules.
Fixes (`#77995 <https://github.com/llvm/llvm-project/issues/77995>`_)

Expand Down
49 changes: 1 addition & 48 deletions clang/lib/AST/ODRHash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -745,55 +745,8 @@ void ODRHash::AddEnumDecl(const EnumDecl *Enum) {
if (Enum->isScoped())
AddBoolean(Enum->isScopedUsingClassTag());

if (Enum->getIntegerTypeSourceInfo()) {
// FIMXE: This allows two enums with different spellings to have the same
// hash.
//
// // mod1.cppm
// module;
// extern "C" {
// typedef unsigned __int64 size_t;
// }
// namespace std {
// using :: size_t;
// }
//
// extern "C++" {
// namespace std {
// enum class align_val_t : std::size_t {};
// }
// }
//
// export module mod1;
// export using std::align_val_t;
//
// // mod2.cppm
// module;
// extern "C" {
// typedef unsigned __int64 size_t;
// }
//
// extern "C++" {
// namespace std {
// enum class align_val_t : size_t {};
// }
// }
//
// export module mod2;
// import mod1;
// export using std::align_val_t;
//
// The above example should be disallowed since it violates
// [basic.def.odr]p14:
//
// Each such definition shall consist of the same sequence of tokens
//
// The definitions of `std::align_val_t` in two module units have different
// spellings but we failed to give an error here.
//
// See https://github.com/llvm/llvm-project/issues/76638 for details.
if (Enum->getIntegerTypeSourceInfo())
AddQualType(Enum->getIntegerType().getCanonicalType());
}

// Filter out sub-Decls which will not be processed in order to get an
// accurate count of Decl's.
Expand Down
51 changes: 51 additions & 0 deletions clang/test/Modules/cxx20-modules-enum-odr.cppm
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// RUN: rm -rf %t
// RUN: mkdir -p %t
// RUN: split-file %s %t
//
// RUN: %clang_cc1 -std=c++20 %t/mod1.cppm -emit-module-interface -o %t/mod1.pcm
// RUN: %clang_cc1 -std=c++20 %t/mod2.cppm -emit-module-interface -o %t/mod2.pcm
// RUN: %clang_cc1 -std=c++20 %t/test.cpp -fprebuilt-module-path=%t -verify -fsyntax-only

//--- size_t.h

extern "C" {
typedef unsigned int size_t;
}

//--- csize_t
namespace std {
using :: size_t;
}

//--- align.h
namespace std {
enum class align_val_t : size_t {};
}

//--- mod1.cppm
module;
#include "size_t.h"
#include "align.h"
export module mod1;
namespace std {
export using std::align_val_t;
}

//--- mod2.cppm
module;
#include "size_t.h"
#include "csize_t"
#include "align.h"
export module mod2;
namespace std {
export using std::align_val_t;
}

//--- test.cpp
// expected-no-diagnostics
import mod1;
import mod2;
void test() {
std::align_val_t v;
}

0 comments on commit 729a0b6

Please sign in to comment.