Skip to content

[clang][bytecode][NFC] Minor cleanups #129553

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

Merged
merged 1 commit into from
Mar 4, 2025
Merged

[clang][bytecode][NFC] Minor cleanups #129553

merged 1 commit into from
Mar 4, 2025

Conversation

tbaederr
Copy link
Contributor

@tbaederr tbaederr commented Mar 3, 2025

Pull local variables in to the closest scope, remove some unnecessary calls to getLocation() and remove an outdated comment.

Pull local variables in to the closest scope, remove some unnecessary
calls to getLocation() and remove an outdated comment.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 3, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 3, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Pull local variables in to the closest scope, remove some unnecessary calls to getLocation() and remove an outdated comment.


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

2 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Interp.cpp (+8-11)
  • (modified) clang/lib/AST/ByteCode/Pointer.cpp (+1-1)
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 5e0d2e91fb1b2..32187e7003c1e 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -551,8 +551,8 @@ bool CheckInitialized(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 
   if (const auto *VD = Ptr.getDeclDesc()->asVarDecl();
       VD && (VD->isConstexpr() || VD->hasGlobalStorage())) {
-    const SourceInfo &Loc = S.Current->getSource(OpPC);
     if (VD->getAnyInitializer()) {
+      const SourceInfo &Loc = S.Current->getSource(OpPC);
       S.FFDiag(Loc, diag::note_constexpr_var_init_non_constant, 1) << VD;
       S.Note(VD->getLocation(), diag::note_declared_at);
     } else {
@@ -722,7 +722,6 @@ bool CheckCallable(InterpState &S, CodePtr OpPC, const Function *F) {
   if (F->isLambdaStaticInvoker())
     return true;
 
-  const SourceLocation &Loc = S.Current->getLocation(OpPC);
   if (S.getLangOpts().CPlusPlus11) {
     const FunctionDecl *DiagDecl = F->getDecl();
 
@@ -748,7 +747,8 @@ bool CheckCallable(InterpState &S, CodePtr OpPC, const Function *F) {
     // or an inheriting constructor, we should be much more explicit about why
     // it's not constexpr.
     if (CD && CD->isInheritingConstructor()) {
-      S.FFDiag(Loc, diag::note_constexpr_invalid_inhctor, 1)
+      S.FFDiag(S.Current->getLocation(OpPC),
+               diag::note_constexpr_invalid_inhctor, 1)
           << CD->getInheritedConstructor().getConstructor()->getParent();
       S.Note(DiagDecl->getLocation(), diag::note_declared_at);
     } else {
@@ -766,7 +766,8 @@ bool CheckCallable(InterpState &S, CodePtr OpPC, const Function *F) {
           DiagDecl->hasBody())
         return false;
 
-      S.FFDiag(Loc, diag::note_constexpr_invalid_function, 1)
+      S.FFDiag(S.Current->getLocation(OpPC),
+               diag::note_constexpr_invalid_function, 1)
           << DiagDecl->isConstexpr() << (bool)CD << DiagDecl;
 
       if (DiagDecl->getDefinition())
@@ -776,7 +777,8 @@ bool CheckCallable(InterpState &S, CodePtr OpPC, const Function *F) {
         S.Note(DiagDecl->getLocation(), diag::note_declared_at);
     }
   } else {
-    S.FFDiag(Loc, diag::note_invalid_subexpr_in_const_expr);
+    S.FFDiag(S.Current->getLocation(OpPC),
+             diag::note_invalid_subexpr_in_const_expr);
   }
 
   return false;
@@ -980,11 +982,6 @@ bool CheckNonNullArgs(InterpState &S, CodePtr OpPC, const Function *F,
   return true;
 }
 
-// FIXME: This is similar to code we already have in Compiler.cpp.
-// I think it makes sense to instead add the field and base destruction stuff
-// to the destructor Function itself. Then destroying a record would really
-// _just_ be calling its destructor. That would also help with the diagnostic
-// difference when the destructor or a field/base fails.
 static bool runRecordDestructor(InterpState &S, CodePtr OpPC,
                                 const Pointer &BasePtr,
                                 const Descriptor *Desc) {
@@ -1094,8 +1091,8 @@ bool Free(InterpState &S, CodePtr OpPC, bool DeleteIsArrayForm,
 
     // For a class type with a virtual destructor, the selected operator delete
     // is the one looked up when building the destructor.
-    QualType AllocType = Ptr.getType();
     if (!DeleteIsArrayForm && !IsGlobalDelete) {
+      QualType AllocType = Ptr.getType();
       auto getVirtualOperatorDelete = [](QualType T) -> const FunctionDecl * {
         if (const CXXRecordDecl *RD = T->getAsCXXRecordDecl())
           if (const CXXDestructorDecl *DD = RD->getDestructor())
diff --git a/clang/lib/AST/ByteCode/Pointer.cpp b/clang/lib/AST/ByteCode/Pointer.cpp
index 92cfa192fd385..e5876409edcf7 100644
--- a/clang/lib/AST/ByteCode/Pointer.cpp
+++ b/clang/lib/AST/ByteCode/Pointer.cpp
@@ -384,7 +384,6 @@ void Pointer::initialize() const {
     return;
 
   assert(PointeeStorage.BS.Pointee && "Cannot initialize null pointer");
-  const Descriptor *Desc = getFieldDesc();
 
   if (isRoot() && PointeeStorage.BS.Base == sizeof(GlobalInlineDescriptor)) {
     GlobalInlineDescriptor &GD = *reinterpret_cast<GlobalInlineDescriptor *>(
@@ -393,6 +392,7 @@ void Pointer::initialize() const {
     return;
   }
 
+  const Descriptor *Desc = getFieldDesc();
   assert(Desc);
   if (Desc->isPrimitiveArray()) {
     // Primitive global arrays don't have an initmap.

@tbaederr tbaederr merged commit 82d111e into llvm:main Mar 4, 2025
14 checks passed
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
Pull local variables in to the closest scope, remove some unnecessary
calls to getLocation() and remove an outdated comment.
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.

2 participants