[NFC] Fix uninitalized member variable use in ASTReader::ParseTargetOptions()

[schittir]

Summary

Found by Coverity static analysis tool.

[tahonermann]

clang/lib/Serialization/ASTReader.cpp

5890–5892 ↗

(On Diff #507221)

It is odd that these values aren't preserved by the AST writer. I wonder if that is intentional.

It took me a while, but I eventually found that these data members are assigned via an inclusion of clang/Driver/Options.inc somewhere.

Rather than assigning values here, I think we would be better off adding member initializers in the class definition. But I think we do need to look into why the AST writer isn't preserving the values.

[tahonermann]

clang/include/clang/Basic/TargetOptions.h

48

Looks good; this is consistent with the default initialization performed for llvm::TargetOptions::EABIVersion.

91

I see that COV_4 is the default as specified in clang/include/clang/Driver/Options.td, but I wonder if we want to duplicate that value here. I'm a bit more inclined to default to COV_None so that we're not trying to maintain the same default in multiple places. I don't know that it matters though.

Perhaps it would make sense to add a "default" option value as is done for EABI? I'm not sure.

[schittir]

Changed CodeObjectVersion default to COV_None

[aaron.ballman]

clang/include/clang/Basic/TargetOptions.h

91

I think none makes the most sense to me. This is set via a CC1 option, and the driver doesn't unconditionally set it (https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L1073), so I think none is safer than assuming CodeObjectVersion is meaningful when it's not set by -cc1. WDYT?

[schittir]

clang/include/clang/Basic/TargetOptions.h

91

I agree, and that's why I made the change to none. @tahonermann

[tahonermann]

Changes look good to me. Thanks, Sindhu!

[schittir]

Thank you, @aaron.ballman and @tahonermann for your comments!
This was committed to https://github.com/llvm/llvm-project.git with hash 6f2a865d2f6bc426a61939a0a1acfcb25d5c1a18

clang/include/clang/Basic/TargetOptions.h

public:
 
   /// If given, the unit to use for floating point math.
   std::string FPMath;
 
   /// If given, the name of the target ABI to use.
   std::string ABI;
 
   /// The EABI version to use
-  llvm::EABI EABIVersion;
+  llvm::EABI EABIVersion = llvm::EABI::Default;
 
   /// If given, the version string of the linker in use.
   std::string LinkerVersion;
 
   /// The list of target specific features to enable or disable, as written on the command line.
   std::vector<std::string> FeaturesAsWritten;
 
   /// The list of target specific features to enable or disable -- this should
public:
   enum CodeObjectVersionKind {
     COV_None,
     COV_2 = 200,
     COV_3 = 300,
     COV_4 = 400,
     COV_5 = 500,
   };
   /// \brief Code object version for AMDGPU.
-  CodeObjectVersionKind CodeObjectVersion;
+  CodeObjectVersionKind CodeObjectVersion = CodeObjectVersionKind::COV_None;
 
   // The code model to be used as specified by the user. Corresponds to
   // CodeModel::Model enum defined in include/llvm/Support/CodeGen.h, plus
   // "default" for the case when the user has not explicitly specified a
   // code model.
   std::string CodeModel;
 
   /// The version of the SDK which was used during the compilation.