[PATCH] D154502: [AST] Fix bug in UnresolvedSet::erase of last element

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.


================
Comment at: clang/unittests/AST/UnresolvedSetTest.cpp:11
+};
+} // namespace clang
+
----------------
jroelofs wrote:
> This ODR violation broke the build for me because it's picking up the definition from Decl.h instead of this one.
> 
> ```
> /Users/jonathan_roelofs/llvm-upstream/clang/unittests/AST/UnresolvedSetTest.cpp:30:1: error: call to implicitly-deleted default constructor of 'UnresolvedSetTest'
> TEST_F(UnresolvedSetTest, Size) { EXPECT_EQ(set.size(), 4u); }
> ^
> /Users/jonathan_roelofs/llvm-upstream/third-party/unittest/googletest/include/gtest/gtest.h:2368:3: note: expanded from macro 'TEST_F'
>   GTEST_TEST_(test_fixture, test_name, test_fixture, \
>   ^
> /Users/jonathan_roelofs/llvm-upstream/third-party/unittest/googletest/include/gtest/internal/gtest-internal.h:1362:5: note: expanded from macro 'GTEST_TEST_'
>     GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)() {}                   \
>     ^
> /Users/jonathan_roelofs/llvm-upstream/third-party/unittest/googletest/include/gtest/internal/gtest-internal.h:1351:3: note: expanded from macro 'GTEST_TEST_CLASS_NAME_'
>   test_suite_name##_##test_name##_Test
>   ^
> <scratch space>:30:1: note: expanded from here
> UnresolvedSetTest_Size_Test
> ^
> /Users/jonathan_roelofs/llvm-upstream/clang/unittests/AST/UnresolvedSetTest.cpp:19:13: note: default constructor of 'UnresolvedSetTest' is implicitly deleted because field 'n0' has no default constructor
>   NamedDecl n0, n1, n2, n3;
>             ^
> ```
> 
> Maybe it needs to be something like this instead?
> 
> ```
> diff --git a/clang/unittests/AST/UnresolvedSetTest.cpp b/clang/unittests/AST/UnresolvedSetTest.cpp
> index ada857e0e382..3c9077ddc3ec 100644
> --- a/clang/unittests/AST/UnresolvedSetTest.cpp
> +++ b/clang/unittests/AST/UnresolvedSetTest.cpp
> @@ -1,22 +1,23 @@
>  #include "clang/AST/UnresolvedSet.h"
> +#include "clang/AST/Decl.h"
>  #include "gtest/gtest.h"
>  
> -namespace clang {
> -class NamedDecl {
> +namespace dummy {
> +class NamedDecl : public clang::NamedDecl {
>    // DeclAccessPair assumes that NamedDecl is at least 4-byte aligned, so we
>    // we need to have a dummy value to make this dummy NamedDecl also be aligned.
>    [[maybe_unused]] int dummy;
>  
>  public:
> -  NamedDecl() {}
> +  NamedDecl() : clang::NamedDecl(clang::NamedDecl::Kind(0), nullptr, clang::SourceLocation::getFromRawEncoding(0), clang::DeclarationName()) {}
>  };
> -} // namespace clang
> +} // namespace dummy
>  
>  using namespace clang;
>  
>  class UnresolvedSetTest : public ::testing::Test {
>  protected:
> -  NamedDecl n0, n1, n2, n3;
> +  dummy::NamedDecl n0, n1, n2, n3;
>    UnresolvedSet<2> set;
>  
>    void SetUp() override {
> 
> ```
You might need to build with `-DLLVM_ENABLE_MODULES=On` to see this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154502/new/

https://reviews.llvm.org/D154502