Don't delete default constructor of PathDiagnosticConsumerOptions

[MoritzS]

Summary

This type is used as an aggregate, i.e. it has no member functions.
Starting with C++20 types with deleted default constructors are not
aggregate types anymore which means that aggregate initialization will
not work for this class anymore. This leads to a compile error in
clang::AnalyzerOptions::getDiagOpts() for example.

Also set the boolean flags to false by default to avoid undefined
behavior. Previously this was prevented by deleting the default
constructor, now we explicitly initialize them.

[NoQ]

Aha, ok then! Thanks for cleaning this up.

Maybe we should provide default initializers for bool flags then, so that they weren't undefined by default.

clang/include/clang/Analysis/PathDiagnostic.h

71

75

85

89

93

96–97

[MoritzS]

Another way to avoid UB in that case is to use value initialization, i.e. PathDiagnosticConsumerOptions options{};.

Can you commit this, please? I don't have commit access.

[MoritzS]

ping

[MoritzS]

Rebased onto master

[MoritzS]

Rebased onto master, run tests again

[vsavchenko]

Hi @MoritzS thanks for the cleanup!
Maybe while you are on it, you can set default values as pointed by @NoQ?

[MoritzS]

Initialize bool flags

[MoritzS]

Some tests were failing, I'll try to fix that and then commit the changes.

clang/include/clang/Analysis/PathDiagnostic.h

 struct PathDiagnosticConsumerOptions {
   /// Run-line of the tool that produced the diagnostic.
   /// It can be included with the diagnostic for debugging purposes.
   std::string ToolInvocation;
 
   /// Whether to include additional information about macro expansions
   /// with the diagnostics, because otherwise they can be hard to obtain
   /// without re-compiling the program under analysis.
-  bool ShouldDisplayMacroExpansions;
+  bool ShouldDisplayMacroExpansions = false;
 
   /// Whether to include LLVM statistics of the process in the diagnostic.
   /// Useful for profiling the tool on large real-world codebases.
-  bool ShouldSerializeStats;
+  bool ShouldSerializeStats = false;
 
   /// If the consumer intends to produce multiple output files, should it
   /// use randomly generated file names for these files (with the tiny risk of
   /// having random collisions) or deterministic human-readable file names
   /// (with a larger risk of deterministic collisions or invalid characters
   /// in the file name). We should not really give this choice to the users
   /// because deterministic mode is always superior when done right, but
   /// for some consumers this mode is experimental and needs to be
   /// off by default.
-  bool ShouldWriteStableReportFilename;
+  bool ShouldWriteStableReportFilename = false;
 
   /// Whether the consumer should treat consumed diagnostics as hard errors.
   /// Useful for breaking your build when issues are found.
-  bool ShouldDisplayWarningsAsErrors;
+  bool ShouldDisplayWarningsAsErrors = false;
 
   /// Whether the consumer should attempt to rewrite the source file
   /// with fix-it hints attached to the diagnostics it consumes.
-  bool ShouldApplyFixIts;
+  bool ShouldApplyFixIts = false;
 
   /// Whether the consumer should present the name of the entity that emitted
   /// the diagnostic (eg., a checker) so that the user knew how to disable it.
-  bool ShouldDisplayDiagnosticName;
-
-  PathDiagnosticConsumerOptions() = delete;
+  bool ShouldDisplayDiagnosticName = false;
 };
 
 class PathDiagnosticConsumer {
 public:
   class PDFileEntry : public llvm::FoldingSetNode {
   public:
     PDFileEntry(llvm::FoldingSetNodeID &NodeID) : NodeID(NodeID) {}