Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][Sema] Diagnosis for constexpr constructor not initializing a union member #81042

Closed
wants to merge 0 commits into from

Conversation

mahtohappy
Copy link
Contributor

@mahtohappy mahtohappy commented Feb 7, 2024

For templates, isDependentContext() is true, that's why the previous if check was failing.
Here, we traverse the RD and upon finding a Union declaration we check if at least one member of the union is initialized.
If not, it produces a diagnosis for it.
Fixes #46689

Copy link

github-actions bot commented Feb 7, 2024

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 llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 7, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 7, 2024

@llvm/pr-subscribers-clang

Author: None (mahtohappy)

Changes

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

3 Files Affected:

  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+19)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp (+3)
  • (added) clang/test/SemaCXX/constexpr-union-temp-ctor-cxx.cpp (+18)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 5adc262cd6bc9..ef4e274389f60 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2393,6 +2393,25 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
                                              Kind))
             return false;
       }
+    } else if(!Constructor->isDelegatingConstructor()){
+      for(const Decl* decl : RD->decls()){
+        if(const auto* inner = dyn_cast<CXXRecordDecl>(decl)){
+          if(inner->isUnion()){
+              if (Constructor->getNumCtorInitializers() == 0 &&
+                RD->hasVariantMembers()) {
+              if (Kind == Sema::CheckConstexprKind::Diagnose) {
+                SemaRef.Diag(
+                    Dcl->getLocation(),
+                    SemaRef.getLangOpts().CPlusPlus20
+                        ? diag::warn_cxx17_compat_constexpr_union_ctor_no_init
+                        : diag::ext_constexpr_union_ctor_no_init);
+              } else if (!SemaRef.getLangOpts().CPlusPlus20) {
+                return false;
+              }
+            }
+          }
+        }
+      }
     }
   } else {
     if (ReturnStmts.empty()) {
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
index f1f677ebfcd34..0d9b4d740a7c1 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
@@ -224,6 +224,9 @@ struct TemplateInit {
   };
   // FIXME: This is ill-formed (no diagnostic required). We should diagnose it.
   constexpr TemplateInit() {} // desired-error {{must initialize all members}}
+#ifndef CXX2A
+  // expected-error@226 {{constexpr union constructor that does not initialize any member is a C++20 extension}}
+#endif
 };
 template<typename T> struct TemplateInit2 {
   Literal l;
diff --git a/clang/test/SemaCXX/constexpr-union-temp-ctor-cxx.cpp b/clang/test/SemaCXX/constexpr-union-temp-ctor-cxx.cpp
new file mode 100644
index 0000000000000..1300641f28f1c
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-union-temp-ctor-cxx.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++14 -verify -fcxx-exceptions -Werror=c++14-extensions -Werror=c++20-extensions %s
+
+template <class> struct C {
+    union {
+      int i;
+    };
+    constexpr C() {} // expected-error {{constexpr union constructor that does not initialize any member is a C++20 extension}}
+};
+constexpr C<int> c;
+
+template <class> class D {
+    union {
+      int i;
+    };
+public:
+    constexpr D() {} // expected-error {{constexpr union constructor that does not initialize any member is a C++20 extension}}
+};
+constexpr D<int> d;
\ No newline at end of file

@mahtohappy mahtohappy changed the title Diagnosis for constexpr constructor not initializing a union member #46689 Diagnosis for constexpr constructor not initializing a union member fixes #46689 Feb 7, 2024
@mahtohappy mahtohappy changed the title Diagnosis for constexpr constructor not initializing a union member fixes #46689 Diagnosis for constexpr constructor not initializing a union member Fixes #46689 Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a24b0c351a75a87410203dd3777c0d8ee87f65c1 45f86f60044dfe5e53782896b7ad450a8817a1f4 -- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p23.cpp clang/test/Sema/conversion-implicit-int-includes-64-to-32.c clang/test/SemaCXX/constexpr-union-temp-ctor-cxx.cpp mlir/unittests/IR/AffineMapTest.cpp clang-tools-extra/clang-move/Move.cpp clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp clang-tools-extra/clangd/Headers.cpp clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/index/IndexAction.cpp clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp clang-tools-extra/include-cleaner/lib/Record.cpp clang-tools-extra/modularize/CoverageChecker.cpp clang-tools-extra/modularize/PreprocessorTracker.cpp clang-tools-extra/pp-trace/PPCallbacksTracker.cpp clang-tools-extra/pp-trace/PPCallbacksTracker.h clang-tools-extra/test/pp-trace/pp-trace-include.cpp clang/include/clang/AST/DeclTemplate.h clang/include/clang/Lex/PPCallbacks.h clang/include/clang/Lex/PreprocessingRecord.h clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h clang/lib/AST/DeclPrinter.cpp clang/lib/AST/ExprConstant.cpp clang/lib/AST/Interp/ByteCodeEmitter.cpp clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/CodeGen/CGBuiltin.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/CodeGen/MacroPPCallbacks.cpp clang/lib/CodeGen/MacroPPCallbacks.h clang/lib/Frontend/DependencyFile.cpp clang/lib/Frontend/DependencyGraph.cpp clang/lib/Frontend/ModuleDependencyCollector.cpp clang/lib/Frontend/PrecompiledPreamble.cpp clang/lib/Frontend/PrintPreprocessedOutput.cpp clang/lib/Frontend/Rewrite/InclusionRewriter.cpp clang/lib/Lex/PPDirectives.cpp clang/lib/Lex/PreprocessingRecord.cpp clang/lib/Parse/ParseOpenACC.cpp clang/lib/Sema/SemaChecking.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp clang/test/AST/Interp/builtin-functions.cpp clang/test/AST/Interp/lambda.cpp clang/test/AST/Interp/records.cpp clang/test/AST/ast-print-method-decl.cpp clang/test/Analysis/Inputs/std-c-library-functions-POSIX.h clang/test/Analysis/std-c-library-functions-POSIX.c clang/test/Analysis/std-c-library-functions.c clang/test/Analysis/stream-error.c clang/test/Analysis/stream-noopen.c clang/test/Analysis/stream.c clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp clang/test/OpenMP/for_loop_auto.cpp clang/test/Sema/conversion-64-32.c clang/tools/libclang/Indexing.cpp clang/unittests/Lex/PPCallbacksTest.cpp flang/include/flang/Lower/ConvertVariable.h flang/include/flang/Optimizer/Builder/HLFIRTools.h flang/lib/Lower/ConvertVariable.cpp flang/lib/Lower/DirectivesCommon.h flang/lib/Lower/OpenACC.cpp flang/lib/Optimizer/Builder/HLFIRTools.cpp flang/lib/Optimizer/Dialect/FIRAttr.cpp flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp flang/lib/Semantics/resolve-names.cpp flang/tools/bbc/bbc.cpp flang/unittests/Optimizer/FortranVariableTest.cpp libcxx/include/__compare/strong_order.h libcxx/include/__compare/weak_order.h libcxx/include/compare libcxx/include/limits libcxx/include/ostream libcxx/include/scoped_allocator libcxx/include/shared_mutex libcxx/include/string libcxx/include/valarray libcxx/include/vector lldb/include/lldb/DataFormatters/FormatManager.h lldb/include/lldb/DataFormatters/TypeSynthetic.h lldb/include/lldb/DataFormatters/VectorIterator.h lldb/include/lldb/lldb-enumerations.h lldb/source/Core/FormatEntity.cpp lldb/source/Core/ValueObjectSyntheticFilter.cpp lldb/source/DataFormatters/FormatManager.cpp lldb/source/DataFormatters/TypeSynthetic.cpp lldb/source/DataFormatters/VectorType.cpp lldb/source/Interpreter/OptionArgParser.cpp lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp lldb/source/Plugins/Language/CPlusPlus/Coroutines.h lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxx.h lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp lldb/source/Plugins/Language/ObjC/Cocoa.cpp lldb/source/Plugins/Language/ObjC/NSArray.cpp lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/Language/ObjC/NSError.cpp lldb/source/Plugins/Language/ObjC/NSException.cpp lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp lldb/source/Plugins/Language/ObjC/NSSet.cpp llvm/include/llvm/ADT/ilist_iterator.h llvm/include/llvm/Analysis/ValueTracking.h llvm/include/llvm/CodeGen/SelectionDAG.h llvm/include/llvm/IR/PatternMatch.h llvm/include/llvm/ObjectYAML/XCOFFYAML.h llvm/lib/Analysis/BasicAliasAnalysis.cpp llvm/lib/Analysis/DomConditionCache.cpp llvm/lib/Analysis/ValueTracking.cpp llvm/lib/CodeGen/CodeGenPrepare.cpp llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp llvm/lib/CodeGen/SelectionDAG/FastISel.cpp llvm/lib/CodeGen/ValueTypes.cpp llvm/lib/IR/BasicBlock.cpp llvm/lib/IR/ConstantRange.cpp llvm/lib/ObjectYAML/XCOFFEmitter.cpp llvm/lib/ObjectYAML/XCOFFYAML.cpp llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.cpp llvm/lib/Target/AArch64/AArch64InstrInfo.cpp llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.cpp llvm/lib/Target/AMDGPU/Disassembler/AMDGPUDisassembler.h llvm/lib/Target/AMDGPU/SIFoldOperands.cpp llvm/lib/Target/DirectX/DXILMetadata.cpp llvm/lib/Target/X86/X86FixupVectorConstants.cpp llvm/lib/Target/X86/X86ISelLowering.cpp llvm/lib/Target/X86/X86InstrInfo.cpp llvm/lib/Target/X86/X86InstrInfo.h llvm/lib/Target/X86/X86MCInstLower.cpp llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp llvm/lib/Transforms/InstCombine/InstCombineInternal.h llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp llvm/lib/Transforms/InstCombine/InstructionCombining.cpp llvm/lib/Transforms/Scalar/SROA.cpp llvm/tools/llc/llc.cpp llvm/tools/llvm-link/llvm-link.cpp llvm/tools/llvm-lto/llvm-lto.cpp llvm/tools/llvm-lto2/llvm-lto2.cpp llvm/tools/llvm-reduce/llvm-reduce.cpp llvm/tools/obj2yaml/xcoff2yaml.cpp llvm/tools/opt/optdriver.cpp llvm/unittests/ADT/IListIteratorBitsTest.cpp llvm/unittests/IR/BasicBlockDbgInfoTest.cpp llvm/unittests/IR/ConstantRangeTest.cpp llvm/unittests/IR/PatternMatch.cpp llvm/unittests/Target/RISCV/RISCVInstrInfoTest.cpp llvm/utils/TableGen/DXILEmitter.cpp mlir/include/mlir/Dialect/Bufferization/IR/BufferizableOpInterface.h mlir/include/mlir/Dialect/Utils/StructuredOpsUtils.h mlir/include/mlir/IR/AffineMap.h mlir/lib/Conversion/TosaToLinalg/TosaToLinalg.cpp mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp mlir/lib/Dialect/Affine/IR/AffineOps.cpp mlir/lib/Dialect/Arith/Transforms/BufferizableOpInterfaceImpl.cpp mlir/lib/Dialect/Bufferization/IR/BufferizableOpInterface.cpp mlir/lib/Dialect/Bufferization/IR/BufferizationOps.cpp mlir/lib/Dialect/Bufferization/Transforms/Bufferize.cpp mlir/lib/Dialect/Bufferization/Transforms/FuncBufferizableOpInterfaceImpl.cpp mlir/lib/Dialect/Linalg/Transforms/Split.cpp mlir/lib/Dialect/Linalg/Utils/Utils.cpp mlir/lib/Dialect/SparseTensor/Transforms/SparseGPUCodegen.cpp mlir/lib/Dialect/SparseTensor/Transforms/Utils/LoopEmitter.cpp mlir/lib/Dialect/SparseTensor/Transforms/Utils/LoopEmitter.h mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.cpp mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorLevel.h mlir/lib/Dialect/Tensor/Transforms/BufferizableOpInterfaceImpl.cpp mlir/lib/Dialect/Vector/IR/VectorOps.cpp mlir/lib/Dialect/Vector/Transforms/LowerVectorContract.cpp mlir/lib/Dialect/Vector/Transforms/VectorTransferSplitRewritePatterns.cpp mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp mlir/lib/IR/AffineMap.cpp mlir/lib/IR/BuiltinTypes.cpp mlir/test/lib/Dialect/Bufferization/TestTensorCopyInsertion.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index a26d2c3ab8..4f838362b7 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -428,9 +428,8 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
       *OS << "#pragma clang module import "
           << SuggestedModule->getFullModuleName(true)
           << " /* clang -E: implicit import for "
-          << "#" << PP.getSpelling(IncludeTok) << " "
-          << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
-          << " */";
+          << "#" << PP.getSpelling(IncludeTok) << " " << (IsAngled ? '<' : '"')
+          << FileName << (IsAngled ? '>' : '"') << " */";
       setEmittedDirectiveOnThisLine();
       break;
 
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8da4f09576..f6c7410e3b 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1795,10 +1795,11 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
                                        Stmt *Body,
                                        Sema::CheckConstexprKind Kind);
 
-static bool checkUnionConstructorIntializer(Sema &SemaRef, const FunctionDecl *Dcl,
-                                       const CXXConstructorDecl *Constructor,
-                                       const CXXRecordDecl *RD,
-                                       Sema::CheckConstexprKind Kind);
+static bool
+checkUnionConstructorIntializer(Sema &SemaRef, const FunctionDecl *Dcl,
+                                const CXXConstructorDecl *Constructor,
+                                const CXXRecordDecl *RD,
+                                Sema::CheckConstexprKind Kind);
 
 // Check whether a function declaration satisfies the requirements of a
 // constexpr function definition or a constexpr constructor definition. If so,
diff --git a/llvm/lib/Target/X86/X86FixupVectorConstants.cpp b/llvm/lib/Target/X86/X86FixupVectorConstants.cpp
index da7dcbb25a..ec366eff39 100644
--- a/llvm/lib/Target/X86/X86FixupVectorConstants.cpp
+++ b/llvm/lib/Target/X86/X86FixupVectorConstants.cpp
@@ -272,9 +272,8 @@ static Constant *rebuildZeroUpperCst(const Constant *C, unsigned NumBits,
   return nullptr;
 }
 
-static Constant *rebuildExtCst(const Constant *C, bool IsSExt,
-                               unsigned NumBits, unsigned NumElts,
-                               unsigned SrcEltBitWidth) {
+static Constant *rebuildExtCst(const Constant *C, bool IsSExt, unsigned NumBits,
+                               unsigned NumElts, unsigned SrcEltBitWidth) {
   unsigned DstEltBitWidth = NumBits / NumElts;
   assert((NumBits % NumElts) == 0 && (NumBits % SrcEltBitWidth) == 0 &&
          (DstEltBitWidth % SrcEltBitWidth) == 0 &&

@mahtohappy mahtohappy changed the title Diagnosis for constexpr constructor not initializing a union member Fixes #46689 Diagnosis for constexpr constructor not initializing a union member Resolves #46689 Feb 7, 2024
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you the PR.

Your summary is blank but should describe the problem you are trying to solve, how the PR solves it and should at the end link to any llvm issues this relates to.

Having solid summaries is critical since this is normally what one sees when using git log and folks should be able to understand the change without having to goto the PR itself or open up the issue that it solves.

Also please add [Clang][Sema] to the front of your title so folks can quickly see what area it applies to.

Without having looked at the fix in detail, this will also require a release note.

@mahtohappy mahtohappy changed the title Diagnosis for constexpr constructor not initializing a union member Resolves #46689 [Clang][Sema] Diagnosis for constexpr constructor not initializing a union member Feb 8, 2024
public:
constexpr D() {} // expected-error {{constexpr union constructor that does not initialize any member is a C++20 extension}}
};
constexpr D<int> d;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a newline at the end of the file.

@@ -224,6 +224,9 @@ struct TemplateInit {
};
// FIXME: This is ill-formed (no diagnostic required). We should diagnose it.
constexpr TemplateInit() {} // desired-error {{must initialize all members}}
#ifndef CXX2A
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We normally do this using parameters to -verify and using those in the expected line e.g.

https://github.com/llvm/llvm-project/blob/main/clang/test/Parser/cxx2b-lambdas-ext-warns.cpp

CC @AaronBallman

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I'm here just following the convention in that file and didn't want to change the run command
https://github.com/llvm/llvm-project/blob/main/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp

Comment on lines 225 to 226
// FIXME: This is ill-formed (no diagnostic required). We should diagnose it.
constexpr TemplateInit() {} // desired-error {{must initialize all members}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've got the desired error, I think :)

Suggested change
// FIXME: This is ill-formed (no diagnostic required). We should diagnose it.
constexpr TemplateInit() {} // desired-error {{must initialize all members}}
constexpr TemplateInit() {}

@@ -150,6 +150,8 @@ Improvements to Clang's diagnostics

- Clang now diagnoses member template declarations with multiple declarators.
- Clang now diagnoses use of the ``template`` keyword after declarative nested name specifiers.
- Clang now diagnoses constexpr constructor for not initializing atleast one member of union
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Clang now diagnoses constexpr constructor for not initializing atleast one member of union
- Clang now diagnoses constexpr constructor for not initializing at least one member of union

int i;
};
public:
constexpr D() {} // expected-error {{constexpr union constructor that does not initialize any member is a C++20 extension}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test checking if we do initialize something there is no error?

@@ -2393,6 +2393,25 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
Kind))
return false;
}
} else if (!Constructor->isDelegatingConstructor()) {
for (const Decl *decl : RD->decls()) {
if (const auto *inner = dyn_cast<CXXRecordDecl>(decl)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-style NIT, the names of the variables should start from a capital.

Suggested change
if (const auto *inner = dyn_cast<CXXRecordDecl>(decl)) {
if (const auto *Inner = dyn_cast<CXXRecordDecl>(Decl)) {

Comment on lines 2400 to 2410
if (Constructor->getNumCtorInitializers() == 0 &&
RD->hasVariantMembers()) {
if (Kind == Sema::CheckConstexprKind::Diagnose) {
SemaRef.Diag(
Dcl->getLocation(),
SemaRef.getLangOpts().CPlusPlus20
? diag::warn_cxx17_compat_constexpr_union_ctor_no_init
: diag::ext_constexpr_union_ctor_no_init);
} else if (!SemaRef.getLangOpts().CPlusPlus20) {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to be copying the check for unions from lines 2346-2354. I also observed that non-template version of the code this patch fixes, works fine now. I wonder, why can't we just do same thing for templated contexts as we do now for non-templated? If that is not possible, because for example it now involves constexpr evaluator and it normally chokes on templates, can we unify the new check with the one from lines 2346-2354?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by unify you mean, put the check in the new function and call it at that place?

@Endilll Endilll removed their request for review February 8, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Constexpr constructor not initializing a union member is not diagnosed
4 participants