[llvm] Support building with c++23

[kykrueger]

closes #154331

This PR addresses all minimum changes needed to compile LLVM and MLIR with the c++23 standard.
It is a work in progress and to be reviewed for better methods of handling the parts of the build broken by c++23.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-tablegen
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-debuginfo

Author: Kyle Krüger (kykrueger)

Changes

closes #154331

This PR addresses all minimum changes needed to compile LLVM and MLIR with the c++23 standard.
It is a work in progress and to be reviewed for better methods of handling the parts of the build broken by c++23.

---

Full diff: https://github.com/llvm/llvm-project/pull/154372.diff

16 Files Affected:

• (modified) llvm/CMakeLists.txt (+1-1)
• (modified) llvm/include/llvm/DebugInfo/GSYM/GsymContext.h (+1)
• (modified) llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h (+1)
• (modified) llvm/include/llvm/DebugInfo/PDB/Native/InputFile.h (+3-3)
• (modified) llvm/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h (+1-1)
• (modified) llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h (+2)
• (modified) llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h (+2)
• (modified) llvm/include/llvm/MC/MCGOFFStreamer.h (+1-3)
• (modified) llvm/lib/DebugInfo/GSYM/GsymContext.cpp (+1)
• (modified) llvm/lib/DebugInfo/PDB/Native/InputFile.cpp (+5)
• (modified) llvm/lib/DebugInfo/PDB/PDBSymbolTypeBuiltin.cpp (+9)
• (modified) llvm/lib/MC/MCGOFFStreamer.cpp (+7)
• (modified) llvm/lib/Remarks/BitstreamRemarkParser.cpp (+2)
• (modified) llvm/lib/Remarks/BitstreamRemarkParser.h (+1-2)
• (modified) llvm/lib/TableGen/TGParser.cpp (+7)
• (modified) llvm/lib/TableGen/TGParser.h (+4-6)

diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index b672cb9365284..133a76c0b6032 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -59,7 +59,7 @@ set(LLVM_LIBDIR_SUFFIX "" CACHE STRING "Define suffix of library directory name
 include(GNUInstallDirs)
 
 # This C++ standard is required to build LLVM.
-set(LLVM_REQUIRED_CXX_STANDARD 17)
+set(LLVM_REQUIRED_CXX_STANDARD 23)
 
 # If we find that the cache contains CMAKE_CXX_STANDARD it means that it's a old CMakeCache.txt
 # and we can just inform the user and then reset it.
diff --git a/llvm/include/llvm/DebugInfo/GSYM/GsymContext.h b/llvm/include/llvm/DebugInfo/GSYM/GsymContext.h
index 9c04ff63c8059..07d599cf9b5c6 100644
--- a/llvm/include/llvm/DebugInfo/GSYM/GsymContext.h
+++ b/llvm/include/llvm/DebugInfo/GSYM/GsymContext.h
@@ -30,6 +30,7 @@ class GsymReader;
 class GsymContext : public DIContext {
 public:
   GsymContext(std::unique_ptr<GsymReader> Reader);
+  ~GsymContext();
 
   GsymContext(GsymContext &) = delete;
   GsymContext &operator=(GsymContext &) = delete;
diff --git a/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h b/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h
index 7e15433b839ed..98f219ae76f31 100644
--- a/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h
+++ b/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h
@@ -10,6 +10,7 @@
 #define LLVM_DEBUGINFO_PDB_IPDBRAWSYMBOL_H
 
 #include "PDBTypes.h"
+#include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/ADT/BitmaskEnum.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/InputFile.h b/llvm/include/llvm/DebugInfo/PDB/Native/InputFile.h
index 0e7b9663f27d2..71df1d59c2177 100644
--- a/llvm/include/llvm/DebugInfo/PDB/Native/InputFile.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/InputFile.h
@@ -55,9 +55,9 @@ class InputFile {
   getOrCreateTypeCollection(TypeCollectionKind Kind);
 
 public:
-  InputFile(PDBFile *Pdb) { PdbOrObj = Pdb; }
-  InputFile(object::COFFObjectFile *Obj) { PdbOrObj = Obj; }
-  InputFile(MemoryBuffer *Buffer) { PdbOrObj = Buffer; }
+  InputFile(PDBFile *Pdb);
+  InputFile(object::COFFObjectFile *Obj);
+  InputFile(MemoryBuffer *Buffer);
   LLVM_ABI ~InputFile();
   InputFile(InputFile &&Other) = default;
 
diff --git a/llvm/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h b/llvm/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h
index d797d00cfa123..a7e8e60af7160 100644
--- a/llvm/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h
+++ b/llvm/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h
@@ -10,6 +10,7 @@
 #define LLVM_DEBUGINFO_PDB_PDBSYMBOLFUNC_H
 
 #include "llvm/DebugInfo/PDB/IPDBRawSymbol.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h"
 #include "llvm/Support/Compiler.h"
 
 #include "PDBSymbol.h"
@@ -21,7 +22,6 @@ namespace pdb {
 
 class PDBSymDumper;
 class PDBSymbolData;
-class PDBSymbolTypeFunctionSig;
 template <typename ChildType> class IPDBEnumChildren;
 
 class LLVM_ABI PDBSymbolFunc : public PDBSymbol {
diff --git a/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h b/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h
index a054b0c02db83..d3bfa00ac9cde 100644
--- a/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h
+++ b/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h
@@ -20,6 +20,8 @@ namespace pdb {
 class LLVM_ABI PDBSymbolTypeBuiltin : public PDBSymbol {
   DECLARE_PDB_SYMBOL_CONCRETE_TYPE(PDB_SymType::BuiltinType)
 public:
+
+  ~PDBSymbolTypeBuiltin();
   void dump(PDBSymDumper &Dumper) const override;
 
   FORWARD_SYMBOL_METHOD(getBuiltinType)
diff --git a/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h b/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h
index 431bf0dab90d9..acc58e10e71c7 100644
--- a/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h
+++ b/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h
@@ -11,9 +11,11 @@
 
 #include "PDBSymbol.h"
 #include "PDBTypes.h"
+#include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/Support/Compiler.h"
 
 #include "llvm/DebugInfo/PDB/IPDBRawSymbol.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeBuiltin.h"
 
 namespace llvm {
 
diff --git a/llvm/include/llvm/MC/MCGOFFStreamer.h b/llvm/include/llvm/MC/MCGOFFStreamer.h
index 6d029f6bd4a29..8888d9e7bdbb3 100644
--- a/llvm/include/llvm/MC/MCGOFFStreamer.h
+++ b/llvm/include/llvm/MC/MCGOFFStreamer.h
@@ -20,9 +20,7 @@ class MCGOFFStreamer : public MCObjectStreamer {
 public:
   MCGOFFStreamer(MCContext &Context, std::unique_ptr<MCAsmBackend> MAB,
                  std::unique_ptr<MCObjectWriter> OW,
-                 std::unique_ptr<MCCodeEmitter> Emitter)
-      : MCObjectStreamer(Context, std::move(MAB), std::move(OW),
-                         std::move(Emitter)) {}
+                 std::unique_ptr<MCCodeEmitter> Emitter);
 
   ~MCGOFFStreamer() override;
 
diff --git a/llvm/lib/DebugInfo/GSYM/GsymContext.cpp b/llvm/lib/DebugInfo/GSYM/GsymContext.cpp
index 18be6d0985462..62b4caa327d87 100644
--- a/llvm/lib/DebugInfo/GSYM/GsymContext.cpp
+++ b/llvm/lib/DebugInfo/GSYM/GsymContext.cpp
@@ -14,6 +14,7 @@
 using namespace llvm;
 using namespace llvm::gsym;
 
+GsymContext::~GsymContext() = default;
 GsymContext::GsymContext(std::unique_ptr<GsymReader> Reader)
     : DIContext(CK_GSYM), Reader(std::move(Reader)) {}
 
diff --git a/llvm/lib/DebugInfo/PDB/Native/InputFile.cpp b/llvm/lib/DebugInfo/PDB/Native/InputFile.cpp
index 328d0f5ab060f..49be0edc33a10 100644
--- a/llvm/lib/DebugInfo/PDB/Native/InputFile.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/InputFile.cpp
@@ -586,3 +586,8 @@ bool llvm::pdb::shouldDumpSymbolGroup(uint32_t Idx, const SymbolGroup &Group,
   // Otherwise, only dump if this is the same module specified.
   return (Filters.DumpModi == Idx);
 }
+llvm::pdb::InputFile::InputFile(PDBFile *Pdb) { PdbOrObj = Pdb; }
+
+llvm::pdb::InputFile::InputFile(object::COFFObjectFile *Obj) { PdbOrObj = Obj; }
+
+llvm::pdb::InputFile::InputFile(MemoryBuffer *Buffer) { PdbOrObj = Buffer; }
diff --git a/llvm/lib/DebugInfo/PDB/PDBSymbolTypeBuiltin.cpp b/llvm/lib/DebugInfo/PDB/PDBSymbolTypeBuiltin.cpp
index eca2a09c1f77b..1c121187c42a8 100644
--- a/llvm/lib/DebugInfo/PDB/PDBSymbolTypeBuiltin.cpp
+++ b/llvm/lib/DebugInfo/PDB/PDBSymbolTypeBuiltin.cpp
@@ -10,9 +10,18 @@
 
 #include "llvm/DebugInfo/PDB/PDBSymDumper.h"
 
+namespace llvm {
+namespace pdb {
+
+PDBSymbolTypeBuiltin::~PDBSymbolTypeBuiltin() = default;
+
+} // namespace pdb
+} // namespace llvm
+
 using namespace llvm;
 using namespace llvm::pdb;
 
+
 void PDBSymbolTypeBuiltin::dump(PDBSymDumper &Dumper) const {
   Dumper.dump(*this);
 }
diff --git a/llvm/lib/MC/MCGOFFStreamer.cpp b/llvm/lib/MC/MCGOFFStreamer.cpp
index 1718e2a4eb2d7..f164c2d7365a8 100644
--- a/llvm/lib/MC/MCGOFFStreamer.cpp
+++ b/llvm/lib/MC/MCGOFFStreamer.cpp
@@ -45,3 +45,10 @@ MCStreamer *llvm::createGOFFStreamer(MCContext &Context,
       new MCGOFFStreamer(Context, std::move(MAB), std::move(OW), std::move(CE));
   return S;
 }
+llvm::MCGOFFStreamer::MCGOFFStreamer(MCContext &Context,
+                                     std::unique_ptr<MCAsmBackend> MAB,
+                                     std::unique_ptr<MCObjectWriter> OW,
+                                     std::unique_ptr<MCCodeEmitter> Emitter)
+    : MCObjectStreamer(Context, std::move(MAB), std::move(OW),
+                       std::move(Emitter)) {}
+
diff --git a/llvm/lib/Remarks/BitstreamRemarkParser.cpp b/llvm/lib/Remarks/BitstreamRemarkParser.cpp
index 312886013598d..20a8ebbadc681 100644
--- a/llvm/lib/Remarks/BitstreamRemarkParser.cpp
+++ b/llvm/lib/Remarks/BitstreamRemarkParser.cpp
@@ -600,3 +600,5 @@ BitstreamRemarkParser::processRemark(BitstreamRemarkParserHelper &Helper) {
 
   return std::move(Result);
 }
+llvm::remarks::BitstreamRemarkParser::BitstreamRemarkParser(StringRef Buf)
+    : RemarkParser(Format::Bitstream), ParserHelper(Buf) {}
diff --git a/llvm/lib/Remarks/BitstreamRemarkParser.h b/llvm/lib/Remarks/BitstreamRemarkParser.h
index f6f79ef199f7e..061206471fee4 100644
--- a/llvm/lib/Remarks/BitstreamRemarkParser.h
+++ b/llvm/lib/Remarks/BitstreamRemarkParser.h
@@ -45,8 +45,7 @@ struct BitstreamRemarkParser : public RemarkParser {
 
   /// Create a parser that expects to find a string table embedded in the
   /// stream.
-  explicit BitstreamRemarkParser(StringRef Buf)
-      : RemarkParser(Format::Bitstream), ParserHelper(Buf) {}
+  explicit BitstreamRemarkParser(StringRef Buf);
 
   Expected<std::unique_ptr<Remark>> next() override;
 
diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index 0c6add59cb282..5d23416b4610b 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -33,6 +33,13 @@ using namespace llvm;
 
 namespace llvm {
 
+RecordsEntry::RecordsEntry(std::unique_ptr<Record> Rec) : Rec(std::move(Rec)) {}
+RecordsEntry::RecordsEntry(std::unique_ptr<ForeachLoop> Loop) : Loop(std::move(Loop)) {}
+RecordsEntry::RecordsEntry(std::unique_ptr<Record::AssertionInfo> Assertion)
+      : Assertion(std::move(Assertion)) {}
+RecordsEntry::RecordsEntry(std::unique_ptr<Record::DumpInfo> Dump)
+      : Dump(std::move(Dump)) {}
+
 struct SubClassReference {
   SMRange RefRange;
   const Record *Rec = nullptr;
diff --git a/llvm/lib/TableGen/TGParser.h b/llvm/lib/TableGen/TGParser.h
index 7edb6c7a9aac6..09b7d5380695d 100644
--- a/llvm/lib/TableGen/TGParser.h
+++ b/llvm/lib/TableGen/TGParser.h
@@ -46,12 +46,10 @@ struct RecordsEntry {
   void dump() const;
 
   RecordsEntry() = default;
-  RecordsEntry(std::unique_ptr<Record> Rec) : Rec(std::move(Rec)) {}
-  RecordsEntry(std::unique_ptr<ForeachLoop> Loop) : Loop(std::move(Loop)) {}
-  RecordsEntry(std::unique_ptr<Record::AssertionInfo> Assertion)
-      : Assertion(std::move(Assertion)) {}
-  RecordsEntry(std::unique_ptr<Record::DumpInfo> Dump)
-      : Dump(std::move(Dump)) {}
+  RecordsEntry(std::unique_ptr<Record> Rec);
+  RecordsEntry(std::unique_ptr<ForeachLoop> Loop);
+  RecordsEntry(std::unique_ptr<Record::AssertionInfo> Assertion);
+  RecordsEntry(std::unique_ptr<Record::DumpInfo> Dump);
 };
 
 /// ForeachLoop - Record the iteration state associated with a for loop.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[nikic]

Can you please provide some more details on why the extra includes are needed?

[kykrueger]

Can you please provide some more details on why the extra includes are needed?

Sure thing, I'll add info as inline comments with the errors that they solve.

[kykrueger]

FAILED: [code=1] lib/DebugInfo/PDB/CMakeFiles/LLVMDebugInfoPDB.dir/PDBSymbol.cpp.o 
/usr/bin/c++ -DGTEST_HAS_RTTI=0 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/Users/kykrueger/repos/llvm-project/build/lib/DebugInfo/PDB -I/Users/kykrueger/repos/llvm-project/llvm/lib/DebugInfo/PDB -I/Users/kykrueger/repos/llvm-project/build/include -I/Users/kykrueger/repos/llvm-project/llvm/include -isystem /opt/homebrew/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -O3 -DNDEBUG -std=c++2b -arch arm64  -fno-exceptions -funwind-tables -fno-rtti -MD -MT lib/DebugInfo/PDB/CMakeFiles/LLVMDebugInfoPDB.dir/PDBSymbol.cpp.o -MF lib/DebugInfo/PDB/CMakeFiles/LLVMDebugInfoPDB.dir/PDBSymbol.cpp.o.d -o lib/DebugInfo/PDB/CMakeFiles/LLVMDebugInfoPDB.dir/PDBSymbol.cpp.o -c /Users/kykrueger/repos/llvm-project/llvm/lib/DebugInfo/PDB/PDBSymbol.cpp
In file included from /Users/kykrueger/repos/llvm-project/llvm/lib/DebugInfo/PDB/PDBSymbol.cpp:9:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbol.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/APFloat.h:19:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/ArrayRef.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/Hashing.h:53:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1846:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/inplace_merge.h:28:
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:79:19: error: invalid application of 'sizeof' to an incomplete type 'llvm::pdb::PDBSymbolTypeFunctionSig'
   79 |     static_assert(sizeof(_Tp) >= 0, "cannot delete an incomplete type");
      |                   ^~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:293:7: note: in instantiation of member function 'std::default_delete<llvm::pdb::PDBSymbolTypeFunctionSig>::operator()' requested here
  293 |       __ptr_.second()(__tmp);
      |       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:262:71: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::PDBSymbolTypeFunctionSig>::reset' requested here
  262 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
      |                                                                       ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolFunc.h:73:3: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::PDBSymbolTypeFunctionSig>::~unique_ptr' requested here
   73 |   FORWARD_CONCRETE_SYMBOL_ID_METHOD_WITH_NAME(PDBSymbolTypeFunctionSig, getType,
      |   ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbol.h:28:12: note: expanded from macro 'FORWARD_CONCRETE_SYMBOL_ID_METHOD_WITH_NAME'
   28 |     return getConcreteSymbolByIdHelper<ConcreteType>(Id);                      \
      |            ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:49:7: note: forward declaration of 'llvm::pdb::PDBSymbolTypeFunctionSig'
   49 | class PDBSymbolTypeFunctionSig;
      |       ^
1 error generated.

The code generated by the macro at line 73:

  FORWARD_CONCRETE_SYMBOL_ID_METHOD_WITH_NAME(PDBSymbolTypeFunctionSig, getType,
                                              getSignature)

Results in a unique_ptr to PDBSymbolTypeFunctionSig which is now a constexpr in c++23.
This constexpr requires that PDBSymbolTypeFunctionSig is a complete type, which was not the case with the forward declaration.

Instead of importing the header here, it should also be possible to move the definition of the methods defined by the macro to the PDBSymbolFunc.cpp file where the complete type is known. Since there are so many methods defined with these macros in PDB, I didn't want to break the paradigm by moving the methods and only declaring them here in the header.

[kykrueger]

In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h:12:                                                                                                      
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbol.h:12:                                                                                                              
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:12:                                                                                                          
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:12:                                                                                                               
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/APFloat.h:19:                                                                                                                          
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/ArrayRef.h:12:                                                                                                                         
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/Hashing.h:53:                                                                                                                          
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1846:                                                                                                           
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/inplace_merge.h:28:                                                                                           
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:79:19: error: invalid application of 'sizeof' to an incomplete type 'llvm::pdb::IPDBLineNumber'                           
   79 |     static_assert(sizeof(_Tp) >= 0, "cannot delete an incomplete type");                                                                                                                                       
      |                   ^~~~~~~~~~~                                                                                                                                                                                  
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:293:7: note: in instantiation of member function 'std::default_delete<llvm::pdb::IPDBLineNumber>::operator()' requested he
re                                                                                                                                                                                                                     
  293 |       __ptr_.second()(__tmp);                                                                                                                                                                                  
      |       ^                                                                                                                                                                                                        
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:262:71: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::IPDBLineNumber>::reset' requested here      
  262 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }                                                                                                                               
      |                                                                       ^                                                                                                                                        
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h:41:25: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::IPDBLineNumber>::~unique_ptr' requested here      
   41 |   FORWARD_SYMBOL_METHOD(getSrcLineOnTypeDefn)                                                                                                                                                                  
      |                         ^                                                                                                                                                                                      
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:30:7: note: forward declaration of 'llvm::pdb::IPDBLineNumber'                                                                          
   30 | class IPDBLineNumber;  

This include solves the above error when compiling against the c++23 standard.

Similar to the case of #include "llvm/DebugInfo/PDB/PDBSymbolTypeFunctionSig.h", I didn't want to break the paradigm of declaring all of the methods with the macros in this header file. It may be possible to fix this by removing the use of this macro FORWARD_SYMBOL_METHOD(getSrcLineOnTypeDefn), and instead only declaring the method while defining it in a place where the full definition of IPDBLineNumber is known.

[kykrueger]

In file included from /Users/kykrueger/repos/llvm-project/llvm/lib/DebugInfo/PDB/PDBSymbolTypeEnum.cpp:9:                                                                                                              
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h:12:                                                                                                      
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbol.h:12:                                                                                                              
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/APFloat.h:19:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/ArrayRef.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/Hashing.h:53:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1846:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/inplace_merge.h:28:
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:79:19: error: invalid application of 'sizeof' to an incomplete type 'llvm::pdb::PDBSymbolTypeBuiltin'
   79 |     static_assert(sizeof(_Tp) >= 0, "cannot delete an incomplete type");
      |                   ^~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:293:7: note: in instantiation of member function 'std::default_delete<llvm::pdb::PDBSymbolTypeBuiltin>::operator()' reques
ted here
  293 |       __ptr_.second()(__tmp);
      |       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:262:71: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::PDBSymbolTypeBuiltin>::reset' requested here
  262 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
      |                                                                       ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h:46:3: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::PDBSymbolTypeBuiltin>::~unique_ptr' requested here
   46 |   FORWARD_CONCRETE_SYMBOL_ID_METHOD_WITH_NAME(PDBSymbolTypeBuiltin, getType,
      |   ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbol.h:28:12: note: expanded from macro 'FORWARD_CONCRETE_SYMBOL_ID_METHOD_WITH_NAME'
   28 |     return getConcreteSymbolByIdHelper<ConcreteType>(Id);                      \
      |            ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:52:7: note: forward declaration of 'llvm::pdb::PDBSymbolTypeBuiltin'
   52 | class PDBSymbolTypeBuiltin;

Similar to the case of #include "llvm/DebugInfo/PDB/IPDBLineNumber.h" above, this include also provides a complete type which is required in the expanded version of a below macro:

  FORWARD_CONCRETE_SYMBOL_ID_METHOD_WITH_NAME(PDBSymbolTypeBuiltin, getType,
                                              getUnderlyingType)

[kykrueger]

eFiles/LLVMDebugInfoPDB.dir/PDBSymbolTypeUDT.cpp.o -c /Users/kykrueger/repos/llvm-project/llvm/lib/DebugInfo/PDB/PDBSymbolTypeUDT.cpp
In file included from /Users/kykrueger/repos/llvm-project/llvm/lib/DebugInfo/PDB/PDBSymbolTypeUDT.cpp:9:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/IPDBRawSymbol.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/APFloat.h:19:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/ArrayRef.h:12:
In file included from /Users/kykrueger/repos/llvm-project/llvm/include/llvm/ADT/Hashing.h:53:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/algorithm:1846:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__algorithm/inplace_merge.h:28:
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:79:19: error: invalid application of 'sizeof' to an incomplete type 'llvm::pdb::IPDBLineNumber'
   79 |     static_assert(sizeof(_Tp) >= 0, "cannot delete an incomplete type");
      |                   ^~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:293:7: note: in instantiation of member function 'std::default_delete<llvm::pdb::IPDBLineNumber>::operator()' requested he
re
  293 |       __ptr_.second()(__tmp);
      |       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:262:71: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::IPDBLineNumber>::reset' requested here
  262 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
      |                                                                       ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h:39:25: note: in instantiation of member function 'std::unique_ptr<llvm::pdb::IPDBLineNumber>::~unique_ptr' requested here
   39 |   FORWARD_SYMBOL_METHOD(getSrcLineOnTypeDefn) 
      |                         ^
/Users/kykrueger/repos/llvm-project/llvm/include/llvm/DebugInfo/PDB/PDBTypes.h:30:7: note: forward declaration of 'llvm::pdb::IPDBLineNumber'
   30 | class IPDBLineNumber;
      |       ^
1 error generated.

Similar to the includes added for the other macros, this import provides a completed class declaration required for a unique_ptr which is generated by FORWARD_SYMBOL_METHOD(getSrcLineOnTypeDefn)

[kykrueger]

Can you please provide some more details on why the extra includes are needed?

The details have been provided in the comments above.
I'm unsure if the additional include statements are really the best way to solve the unique_ptr errors in the long run, or if this justifies splitting out all methods and constructors that use unique_ptr into the respective .cpp files.

[kykrueger]

I've pushed all required patches to build llvm and mlir with c++23 as the standard.

[frederick-vs-ja]

These changes seem largely related to the addition of constexpr to unique_ptr in C++23, which caused Clang to require type completeness in more cases.

I guess the completeness requirement is somehow superfluous, see #59292. Perhaps we should mention the that issue in the PR description (which will be used for squashed commit message by default).

[kykrueger]

Exactly, clang is a bit eager in instantiating the unique_ptr usage in header files with the constexpr property of c++23.
This is related to #5929 as it is a similar error that was then showing up when constexpr changes with vectors and strings that were added in c+20.
With respect to the errors solved in this PR, the first mention I can find of errors due to the instantiation time of constexpr unique_ptrs is found in #74963.

In particular, this comment describes largest problem addressed in this PR: #74963 (comment)

Referencing the above comment is an example of this issue in another project, which appears to be using MCSV. It looks like these changes with respect to unique_ptr in c++23 have broken a popular paradigm of defining some methods in header files use unique_pointers to hold references to imported classes and methods.

[nikic]

If we're working around a clang bug here, can we fix clang instead?

[kykrueger]

From what I've found in other issues, it seems that clang isn't wrong to be instantiating the unique_ptrs as early as it is. I'll see what else I can find on the topic.

[kykrueger]

Here is another mention of this issue from a different project. The consensus there is that the other compilers are allowing invalid c++ code according to the standard, and that . So rather I'd say that these errors are the result of a bug-fix.

As commented in the linked issue:

If fact cppreference.com states that:

std::unique_ptr may be constructed for an incomplete type T, such as to facilitate the use as a handle in the pImpl idiom. If the default deleter is used, T must be complete at the point in code where the deleter is invoked, which happens in the destructor, move assignment operator, and reset member function of std::unique_ptr.

[kykrueger]

In this comment it also seems like the clang team does not intend on changing their implementation to avoid such errors:

This would seem to require that we instantiate constexpr functions on-demand, when they are actually called during constant evaluation, rather than performing the instantiations in advance of constant evaluation[1]. This has various problems:

• It breaks layering: constant evaluation needs template instantiation and hence all of Sema. This is a problem for uses of the Clang AST library, which is not allowed to depend on Sema but contains constant evaluation. For example, this is a problem for code generation, which should not assume there's still a Sema instance around but needs to perform constant evaluation. It's probably feasible to handle this, by adding an optional callback from constant evaluation into template instantiation, and only providing it in the cases of language-semantics-required constant evaluations performed from Sema.

• It breaks instantiation depth: during a deeply-recursive constant evaluation, we would be able to trigger a template instantiation, which could result in a deeply-recursive constant evaluation, that then triggers another template instantiation, and so on. This dramatically increases the potential stack depth per template instantiation "step", which seems likely to run us out of stack in a way that our existing guards against this problem won't catch.

• It breaks the desirable language property that constant evaluation is free of side-effects. If constant evaluation can trigger template instantiation, then it becomes observable and whether a certain subexpression was evaluated can have effects on the behavior of the program. This is also mitigated to some extent by only triggering template instantiation from constant evaluations that are required by the language semantics, so that speculative evaluations used to drive warnings, and enabling warning flags, don't change the program behavior, but will require significant caution.

This is not to say that we shouldn't do it, but it's not trivial, and these concerns led to the language rules explicitly accommodating implementations that want to eagerly instantiate constexpr functions on first use.

[1]: I think there is actually another strategy that we could take. We could track a call graph of constexpr functions, like we do for CUDA, then when Sema is about to kick off a language-semantics-required constant evaluation, it could quickly scan the expression for function references and instantiate those functions plus anything reachable from them in the call graph that is not yet instantiated. That would avoid the instantiation depth problem and the layering problem, at least, and because it'd be an explicit step taken by Sema it'd be less likely that an implementation of a warning flag would accidentally trigger template instantiation. But it'd come with additional storage costs.

Originally posted by @zygoloid in #59966

[nikic]

@AaronBallman Could you please take a look at this? Is this being "Won't Fix" still the current stance of Clang? This seems like a bad footgun for adoption of future C++ standards :(

[shafik]

I am not necessarily arguing against this but I suspect if we do look into this, it won't be a quick fix and we will likely run into a lot of core issues along the way. I don't see this being resolved quickly.

One also has to consider, we eventually need someone to implement reflection and that is likely going to be a large set of refactoring and a core issue generator galore. Likely the refactoring and core issues will kind of all be around the same areas. So we may be pretty resource constrained once this work starts.

BTW Aaron is out for the next month.

[kykrueger]

Oh, I mistakenly pressed the "Update branch" button. Feel free to force-push to fix the commit history.

(Formatting errors still need to be resolved.)

Looks like clang-format was missing from my system.
They should all be resolved now.

[kykrueger]

Ping
@aganea @nikic @JDevlieghere

Could you give a review or let me know what next steps are needed before being able to get this merged?
I'm keen to avoid merge conflicts from showing up

[nikic]

LGTM with the unrelated get() changes dropped.

I can't say I like this, but I guess we don't really have a choice :(

[kykrueger]

@nikic @frederick-vs-ja

I've removed the .get() method calls from the Conditions.
It is indeed unfortunate that these changes related to the unique_ptrs are just to accomodate side-effects of the c++23 standard and don't bring a real benefit aside from letting us build against it.

If you have other ideas to get this working I'm happy to take a look.

[github-actions]

@kykrueger Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

[kykrueger]

Thanks @nikic

[kykrueger]

I'd like to back port this PR to 21.x.x, but do not have the required permissions to add a milestone tag.
Could someone please help with this?

[kykrueger]

@nikic in the contribution docs it says I should ask someone from the project for support in getting this tagged for back-porting a PR that has been merged. Would you be able to help with that, or suggest someone who could?

[frederick-vs-ja]

/cherry-pick 5d0294f

[llvmbot]

/pull-request #162510