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][NFC] constify or staticify some CGRecordLowering fns #82874

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

urnathan
Copy link
Contributor

As mentioned in PR #65742, these functions do not alter the object -- and on one case don't even need it. Thus marking them static or const as appropriate. I constified a few more than I'd originally fallen over.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:codegen labels Feb 24, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 24, 2024

@llvm/pr-subscribers-clang

Author: Nathan Sidwell (urnathan)

Changes

As mentioned in PR #65742, these functions do not alter the object -- and on one case don't even need it. Thus marking them static or const as appropriate. I constified a few more than I'd originally fallen over.


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGRecordLayoutBuilder.cpp (+14-14)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868ef810f3c4e8..7822903b89ce47 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -95,7 +95,7 @@ struct CGRecordLowering {
   CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D, bool Packed);
   // Short helper routines.
   /// Constructs a MemberInfo instance from an offset and llvm::Type *.
-  MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
+  static MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
     return MemberInfo(Offset, MemberInfo::Field, Data);
   }
 
@@ -104,7 +104,7 @@ struct CGRecordLowering {
   /// fields of the same formal type.  We want to emit a layout with
   /// these discrete storage units instead of combining them into a
   /// continuous run.
-  bool isDiscreteBitFieldABI() {
+  bool isDiscreteBitFieldABI() const {
     return Context.getTargetInfo().getCXXABI().isMicrosoft() ||
            D->isMsStruct(Context);
   }
@@ -121,22 +121,22 @@ struct CGRecordLowering {
   /// other bases, which complicates layout in specific ways.
   ///
   /// Note specifically that the ms_struct attribute doesn't change this.
-  bool isOverlappingVBaseABI() {
+  bool isOverlappingVBaseABI() const {
     return !Context.getTargetInfo().getCXXABI().isMicrosoft();
   }
 
   /// Wraps llvm::Type::getIntNTy with some implicit arguments.
-  llvm::Type *getIntNType(uint64_t NumBits) {
+  llvm::Type *getIntNType(uint64_t NumBits) const {
     unsigned AlignedBits = llvm::alignTo(NumBits, Context.getCharWidth());
     return llvm::Type::getIntNTy(Types.getLLVMContext(), AlignedBits);
   }
   /// Get the LLVM type sized as one character unit.
-  llvm::Type *getCharType() {
+  llvm::Type *getCharType() const {
     return llvm::Type::getIntNTy(Types.getLLVMContext(),
                                  Context.getCharWidth());
   }
   /// Gets an llvm type of size NumChars and alignment 1.
-  llvm::Type *getByteArrayType(CharUnits NumChars) {
+  llvm::Type *getByteArrayType(CharUnits NumChars) const {
     assert(!NumChars.isZero() && "Empty byte arrays aren't allowed.");
     llvm::Type *Type = getCharType();
     return NumChars == CharUnits::One() ? Type :
@@ -144,7 +144,7 @@ struct CGRecordLowering {
   }
   /// Gets the storage type for a field decl and handles storage
   /// for itanium bitfields that are smaller than their declared type.
-  llvm::Type *getStorageType(const FieldDecl *FD) {
+  llvm::Type *getStorageType(const FieldDecl *FD) const {
     llvm::Type *Type = Types.ConvertTypeForMem(FD->getType());
     if (!FD->isBitField()) return Type;
     if (isDiscreteBitFieldABI()) return Type;
@@ -152,29 +152,29 @@ struct CGRecordLowering {
                              (unsigned)Context.toBits(getSize(Type))));
   }
   /// Gets the llvm Basesubobject type from a CXXRecordDecl.
-  llvm::Type *getStorageType(const CXXRecordDecl *RD) {
+  llvm::Type *getStorageType(const CXXRecordDecl *RD) const {
     return Types.getCGRecordLayout(RD).getBaseSubobjectLLVMType();
   }
-  CharUnits bitsToCharUnits(uint64_t BitOffset) {
+  CharUnits bitsToCharUnits(uint64_t BitOffset) const {
     return Context.toCharUnitsFromBits(BitOffset);
   }
-  CharUnits getSize(llvm::Type *Type) {
+  CharUnits getSize(llvm::Type *Type) const {
     return CharUnits::fromQuantity(DataLayout.getTypeAllocSize(Type));
   }
-  CharUnits getAlignment(llvm::Type *Type) {
+  CharUnits getAlignment(llvm::Type *Type) const {
     return CharUnits::fromQuantity(DataLayout.getABITypeAlign(Type));
   }
-  bool isZeroInitializable(const FieldDecl *FD) {
+  bool isZeroInitializable(const FieldDecl *FD) const {
     return Types.isZeroInitializable(FD->getType());
   }
-  bool isZeroInitializable(const RecordDecl *RD) {
+  bool isZeroInitializable(const RecordDecl *RD) const {
     return Types.isZeroInitializable(RD);
   }
   void appendPaddingBytes(CharUnits Size) {
     if (!Size.isZero())
       FieldTypes.push_back(getByteArrayType(Size));
   }
-  uint64_t getFieldBitOffset(const FieldDecl *FD) {
+  uint64_t getFieldBitOffset(const FieldDecl *FD) const {
     return Layout.getFieldOffset(FD->getFieldIndex());
   }
   // Layout routines.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 24, 2024

@llvm/pr-subscribers-clang-codegen

Author: Nathan Sidwell (urnathan)

Changes

As mentioned in PR #65742, these functions do not alter the object -- and on one case don't even need it. Thus marking them static or const as appropriate. I constified a few more than I'd originally fallen over.


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

1 Files Affected:

  • (modified) clang/lib/CodeGen/CGRecordLayoutBuilder.cpp (+14-14)
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 868ef810f3c4e8..7822903b89ce47 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -95,7 +95,7 @@ struct CGRecordLowering {
   CGRecordLowering(CodeGenTypes &Types, const RecordDecl *D, bool Packed);
   // Short helper routines.
   /// Constructs a MemberInfo instance from an offset and llvm::Type *.
-  MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
+  static MemberInfo StorageInfo(CharUnits Offset, llvm::Type *Data) {
     return MemberInfo(Offset, MemberInfo::Field, Data);
   }
 
@@ -104,7 +104,7 @@ struct CGRecordLowering {
   /// fields of the same formal type.  We want to emit a layout with
   /// these discrete storage units instead of combining them into a
   /// continuous run.
-  bool isDiscreteBitFieldABI() {
+  bool isDiscreteBitFieldABI() const {
     return Context.getTargetInfo().getCXXABI().isMicrosoft() ||
            D->isMsStruct(Context);
   }
@@ -121,22 +121,22 @@ struct CGRecordLowering {
   /// other bases, which complicates layout in specific ways.
   ///
   /// Note specifically that the ms_struct attribute doesn't change this.
-  bool isOverlappingVBaseABI() {
+  bool isOverlappingVBaseABI() const {
     return !Context.getTargetInfo().getCXXABI().isMicrosoft();
   }
 
   /// Wraps llvm::Type::getIntNTy with some implicit arguments.
-  llvm::Type *getIntNType(uint64_t NumBits) {
+  llvm::Type *getIntNType(uint64_t NumBits) const {
     unsigned AlignedBits = llvm::alignTo(NumBits, Context.getCharWidth());
     return llvm::Type::getIntNTy(Types.getLLVMContext(), AlignedBits);
   }
   /// Get the LLVM type sized as one character unit.
-  llvm::Type *getCharType() {
+  llvm::Type *getCharType() const {
     return llvm::Type::getIntNTy(Types.getLLVMContext(),
                                  Context.getCharWidth());
   }
   /// Gets an llvm type of size NumChars and alignment 1.
-  llvm::Type *getByteArrayType(CharUnits NumChars) {
+  llvm::Type *getByteArrayType(CharUnits NumChars) const {
     assert(!NumChars.isZero() && "Empty byte arrays aren't allowed.");
     llvm::Type *Type = getCharType();
     return NumChars == CharUnits::One() ? Type :
@@ -144,7 +144,7 @@ struct CGRecordLowering {
   }
   /// Gets the storage type for a field decl and handles storage
   /// for itanium bitfields that are smaller than their declared type.
-  llvm::Type *getStorageType(const FieldDecl *FD) {
+  llvm::Type *getStorageType(const FieldDecl *FD) const {
     llvm::Type *Type = Types.ConvertTypeForMem(FD->getType());
     if (!FD->isBitField()) return Type;
     if (isDiscreteBitFieldABI()) return Type;
@@ -152,29 +152,29 @@ struct CGRecordLowering {
                              (unsigned)Context.toBits(getSize(Type))));
   }
   /// Gets the llvm Basesubobject type from a CXXRecordDecl.
-  llvm::Type *getStorageType(const CXXRecordDecl *RD) {
+  llvm::Type *getStorageType(const CXXRecordDecl *RD) const {
     return Types.getCGRecordLayout(RD).getBaseSubobjectLLVMType();
   }
-  CharUnits bitsToCharUnits(uint64_t BitOffset) {
+  CharUnits bitsToCharUnits(uint64_t BitOffset) const {
     return Context.toCharUnitsFromBits(BitOffset);
   }
-  CharUnits getSize(llvm::Type *Type) {
+  CharUnits getSize(llvm::Type *Type) const {
     return CharUnits::fromQuantity(DataLayout.getTypeAllocSize(Type));
   }
-  CharUnits getAlignment(llvm::Type *Type) {
+  CharUnits getAlignment(llvm::Type *Type) const {
     return CharUnits::fromQuantity(DataLayout.getABITypeAlign(Type));
   }
-  bool isZeroInitializable(const FieldDecl *FD) {
+  bool isZeroInitializable(const FieldDecl *FD) const {
     return Types.isZeroInitializable(FD->getType());
   }
-  bool isZeroInitializable(const RecordDecl *RD) {
+  bool isZeroInitializable(const RecordDecl *RD) const {
     return Types.isZeroInitializable(RD);
   }
   void appendPaddingBytes(CharUnits Size) {
     if (!Size.isZero())
       FieldTypes.push_back(getByteArrayType(Size));
   }
-  uint64_t getFieldBitOffset(const FieldDecl *FD) {
+  uint64_t getFieldBitOffset(const FieldDecl *FD) const {
     return Layout.getFieldOffset(FD->getFieldIndex());
   }
   // Layout routines.

@urnathan urnathan changed the title [clang][NFC] constify or staticify some fns [clang][NFC] constify or staticify some CGRecordLowering fns Feb 24, 2024
These functions do not alter the object -- and on one case don't even
need it. Thus marking them static or const as appropriate.
Copy link
Collaborator

@efriedma-quic efriedma-quic left a comment

Choose a reason for hiding this comment

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

I'm not sure there's really much point to passing around a const reference to a CGRecordLowering; all the uses are in one file, so const-correctness doesn't seem to help that much.

But if you want to do it this way, that's fine; LGTM

@urnathan urnathan merged commit cb2dd02 into llvm:main Feb 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants