[clang] fixes named return of variables with dependent alignment

[mizvekov]

Summary

Named return of a variable with aligned attribute would
trip an assert in case alignment was dependent.

Signed-off-by: Matheus Izvekov <[e-mail]>

[aaronpuchert]

clang/lib/Sema/SemaStmt.cpp

3401

Perhaps use std::any_of? Maybe there is even a range wrapper in llvm.

[mizvekov]

• Add tests.
• Refactor common users of this new utility.
• Use std::any_of.

[• Quuxplusone]

clang/lib/Sema/SemaDecl.cpp

13316

Tangent: It would be nice to rename isAlignmentDependent into hasDependentAlignment or simply isDependent. IIUC, the intended meaning is "Is this alignment attribute 'dependent'?" and not (as the name would naturally parse) "is this alignment attribute 'alignment-dependent'?"

Arguably, it could also work to say I->isDependentAlignment() in the same way that we say getType()->isDependentType().

Speaking of which, maybe I should be renamed to AA or something? Is I really the right abbreviation for an AlignedAttr?

[mizvekov]

• Rename predicate variable from I to AA.

[mizvekov]

clang/lib/Sema/SemaDecl.cpp

13316

I agree, but it is not so simple. This attribute class is autogenerated from clang/include/clang/Basic/Attr.td and it's not obvious to me where that method name comes from. Probably the tablegen backend. But that is another area that I am not familiar with yet.
And there are a few users, and also a method named isAlignmentErrorDependent which would benefit from the same kind of rename.
This DR is supposed to be a quick fix for a regression, I better leave that tangent for another time :)

The second rename, well it is very common to name a predicate variable as I, but I renamed to AA as that is also good.

[mizvekov]

format.

[aaronpuchert]

I'd leave it to @rsmith to decide whether this makes sense in Sema.h.

I tend to follow the rule that if it's just used once I keep it local, but you might have good reasons for making it public.

clang/lib/Sema/SemaDecl.cpp

13315

From llvm/include/llvm/ADT/STLExtras.h.

13316

Seems plausible that it comes from

let Args = [ExprArgument<"Alignment">];

in AlignValue. An expression can be dependent, and to distinguish expression arguments, it probably generates functions isXXXDependent for ExprArgument<"XXX">. But I'm just guessing.

But yeah, nothing we can easily change here.

[mizvekov]

In D105380#2858638, @aaronpuchert wrote:

I tend to follow the rule that if it's just used once I keep it local, but you might have good reasons for making it public.

Well it is used twice.
Notice how I replaced a pre-existing static function called hasDependentAlignment with a static class member with the same name.
Since I kept the name, and it was used from a Sema member function, I did not have to change the code in the call site, hence it did not show up in the diff :)

clang/lib/Sema/SemaDecl.cpp

13315

That looks much better, thanks!

[mizvekov]

• Use llvm::any_of.

[rsmith]

clang/lib/Sema/SemaDecl.cpp

13312–13313

It doesn't look like this has any particular relationship to Sema. Consider making this a member of VarDecl instead.

[mizvekov]

• Move it to VarDecl.

[MForster]

Adrian has reverted this commit for me. Here is a reproduction case for the issue that is triggered by this change:

#include <initializer_list>

template<typename T>
struct S {};

template<typename T>  
S<T> foo() {
  for (auto status : { S<T>() }) {
      return status;
  }
}

void a() {
  foo<int>();
}

This is the segmentation fault:

instantiating function definition 'foo<int>'
 #0 0x000055807ba49925 SignalHandler(int) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x6649925)
 #1 0x00007f18a9d509a0 __restore_rt (/usr/grte/v4/lib64/libpthread.so.0+0xf9a0)
 #2 0x000055807901a01d clang::ASTContext::getTypeInfoImpl(clang::Type const*) const (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x3c1a01d)
 #3 0x0000558078ffcf93 clang::ASTContext::getPreferredTypeAlign(clang::Type const*) const (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x3bfcf93)
 #4 0x0000558078ffc4ea clang::ASTContext::getDeclAlign(clang::Decl const*, bool) const (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x3bfc4ea)
 #5 0x0000558078029686 clang::Sema::getNamedReturnInfo(clang::VarDecl const*) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2c29686)
 #6 0x0000558078026e84 clang::TemplateDeclInstantiator::VisitVarDecl(clang::VarDecl*, bool, llvm::ArrayRef<clang::BindingDecl*>*) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2c26e84)
 #7 0x0000558079388ebc clang::Sema::SubstDecl(clang::Decl*, clang::DeclContext*, clang::MultiLevelTemplateArgumentList const&) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x3f88ebc)
 #8 0x0000558077fd8a2d clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformDeclStmt(clang::DeclStmt*) (.llvm.3943398439186356346) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2bd8a2d)
 #9 0x000055807823f79f clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCXXForRangeStmt(clang::CXXForRangeStmt*) (.llvm.3943398439186356346) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2e3f79f)
#10 0x00005580794bfa18 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*) (.llvm.3943398439186356346) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x40bfa18)
#11 0x000055807940149f clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool, bool) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x400149f)
#12 0x00005580794027a3 clang::Sema::PerformPendingInstantiations(bool) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x40027a3)
#13 0x00005580780f5443 clang::Sema::ActOnEndOfTranslationUnitFragment(clang::Sema::TUFragmentKind) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2cf5443)
#14 0x00005580780f461c clang::Sema::ActOnEndOfTranslationUnit() (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2cf461c)
#15 0x00005580785530e3 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, bool) (.cold) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x31530e3)
#16 0x00005580780f224f clang::ParseAST(clang::Sema&, bool, bool) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2cf224f)
#17 0x00005580781fff95 clang::FrontendAction::Execute() (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2dfff95)
#18 0x00005580781ffdc4 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2dffdc4)
#19 0x00005580781ffc98 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2dffc98)
#20 0x000055807974f570 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x434f570)
#21 0x00005580782a04b9 ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2ea04b9)
#22 0x000055807964f49b main (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x424f49b)
#23 0x00007f18a9bbebbd __libc_start_main (/usr/grte/v4/lib64/libc.so.6+0x38bbd)
#24 0x0000558077fc92c9 _start (third_party/crosstool/v18/llvm_unstable/toolchain/bin/clang+0x2bc92c9)

[mizvekov]

Thanks for reporting that one!
Yeah looking if the type is dependent is not enough, a non-deduced auto type is not considered dependent.
I will work on a patch for it later today.

[mizvekov]

• hasDependentAlignment now returns true for VarDecls with undeduced auto types.
• Add couple of tests that cover these kinds of VarDecls appearing:
  • Valid code with a range for loop in dependent context.
  • Error recovery on invalid initialization of auto variable in dependent context.

Remove isInvalidDecl check on FinalizeDeclaration for alignment of TLS variables,
as this is covered by the undeduced auto check above.

[mizvekov]

.

clang/include/clang/AST/Decl.h

public:
   /// Indicates the capture is a __block variable that is never captured by an
   /// escaping block.
   bool isNonEscapingByref() const;
 
   void setEscapingByref() {
     NonParmVarDeclBits.EscapingByref = true;
   }
 
+  /// Determines if this variable's alignment is dependent.
+  bool hasDependentAlignment() const;
+
   /// Retrieve the variable declaration from which this variable could
   /// be instantiated, if it is an instantiation (rather than a non-template).
   VarDecl *getTemplateInstantiationPattern() const;
 
   /// If this variable is an instantiated static data member of a
   /// class template specialization, returns the templated static data member
   /// from which it was instantiated.
   VarDecl *getInstantiatedFromStaticDataMember() const;

clang/lib/AST/Decl.cpp

 bool VarDecl::isEscapingByref() const {
   return hasAttr<BlocksAttr>() && NonParmVarDeclBits.EscapingByref;
 }
 
 bool VarDecl::isNonEscapingByref() const {
   return hasAttr<BlocksAttr>() && !NonParmVarDeclBits.EscapingByref;
 }
 
+bool VarDecl::hasDependentAlignment() const {
+  QualType T = getType();
+  return T->isDependentType() || T->isUndeducedAutoType() ||
+         llvm::any_of(specific_attrs<AlignedAttr>(), [](const AlignedAttr *AA) {
+           return AA->isAlignmentDependent();
+         });
+}
+
 VarDecl *VarDecl::getTemplateInstantiationPattern() const {
   const VarDecl *VD = this;
 
   // If this is an instantiated member, walk back to the template from which
   // it was instantiated.
   if (MemberSpecializationInfo *MSInfo = VD->getMemberSpecializationInfo()) {
     if (isTemplateInstantiation(MSInfo->getTemplateSpecializationKind())) {
       VD = VD->getInstantiatedFromStaticDataMember();

clang/lib/Sema/SemaDecl.cpp

void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
   // module.
   if (Context.DeclMustBeEmitted(var) && !ModuleScopes.empty())
     Context.addModuleInitializer(ModuleScopes.back().Module, var);
 
   // Build the bindings if this is a structured binding declaration.
   if (auto *DD = dyn_cast<DecompositionDecl>(var))
     CheckCompleteDecompositionDeclaration(DD);
 }
 
-/// Determines if a variable's alignment is dependent.
-static bool hasDependentAlignment(VarDecl *VD) {
-  if (VD->getType()->isDependentType())
-    return true;
-  for (auto *I : VD->specific_attrs<AlignedAttr>())
-    if (I->isAlignmentDependent())
-      return true;
-  return false;
-}
-
 /// Check if VD needs to be dllexport/dllimport due to being in a
 /// dllexport/import function.
 void Sema::CheckStaticLocalForDllExport(VarDecl *VD) {
   assert(VD->isStaticLocal());
 
   auto *FD = dyn_cast_or_null<FunctionDecl>(VD->getParentFunctionOrMethod());
 
   // Find outermost function when VD is in lambda function.
   while (FD && !getDLLAttr(FD) &&
          !FD->hasAttr<DLLExportStaticLocalAttr>() &&
          !FD->hasAttr<DLLImportStaticLocalAttr>()) {
     FD = dyn_cast_or_null<FunctionDecl>(FD->getParentFunctionOrMethod());
void Sema::FinalizeDeclaration(Decl *ThisDecl) {
   checkAttributesAfterMerging(*this, *VD);
 
   // Perform TLS alignment check here after attributes attached to the variable
   // which may affect the alignment have been processed. Only perform the check
   // if the target has a maximum TLS alignment (zero means no constraints).
   if (unsigned MaxAlign = Context.getTargetInfo().getMaxTLSAlign()) {
     // Protect the check so that it's not performed on dependent types and
     // dependent alignments (we can't determine the alignment in that case).
-    if (VD->getTLSKind() && !hasDependentAlignment(VD) &&
-        !VD->isInvalidDecl()) {
+    if (VD->getTLSKind() && !VD->hasDependentAlignment()) {
       CharUnits MaxAlignChars = Context.toCharUnitsFromBits(MaxAlign);
       if (Context.getDeclAlign(VD) > MaxAlignChars) {
         Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
           << (unsigned)Context.getDeclAlign(VD).getQuantity() << VD
           << (unsigned)MaxAlignChars.getQuantity();
       }
     }
   }

clang/lib/Sema/SemaStmt.cpp

if (VDReferencedType.isVolatileQualified() ||
       return NamedReturnInfo();
     Info.S = NamedReturnInfo::MoveEligible;
   } else {
     return NamedReturnInfo();
   }
 
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
-  if (!VDType->isDependentType() && VD->hasAttr<AlignedAttr>() &&
+  if (!VD->hasDependentAlignment() &&
       Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VDType))
     Info.S = NamedReturnInfo::MoveEligible;
 
   return Info;
 }
 
 /// Updates given NamedReturnInfo's move-eligible and
 /// copy-elidable statuses, considering the function
 /// return type criteria as applicable to return statements.
 ///
 /// \param Info The NamedReturnInfo object to update.

clang/test/CXX/class/class.init/class.copy.elision/p3.cpp

 
 void test5() try {
   CopyOnly x;
   throw x; // cxx2b-error {{no matching constructor for initialization}}
 } catch (...) {
 }
 
 } // namespace test_simpler_implicit_move
+
+namespace test_auto_variables {
+
+template<class T> struct X {};
+
+template<class T> struct range {
+  X<T> *begin() const;
+  X<T> *end() const;
+};
+
+template<class T> X<T> test_dependent_ranged_for() {
+  for(auto x : range<T>())
+    return x;
+  return X<T>();
+}
+template X<int> test_dependent_ranged_for<int>();
+
+template<class T> X<T> test_dependent_invalid_decl() {
+  auto x = X<T>().foo(); // expected-error {{no member named 'foo'}}
+  return x;
+}
+template X<int> test_dependent_invalid_decl<int>(); // expected-note {{requested here}}
+
+} // namespace test_auto_variables

clang/test/CodeGen/nrvo-tracking.cpp

 // RUN: %clang_cc1 -std=c++20 -fblocks -Wno-return-stack-address -triple x86_64-unknown-unknown-gnu -emit-llvm -O1 -fexperimental-new-pass-manager -o - %s | FileCheck %s
 
-struct X {
-    X();
-    X(const X&);
-    X(X&&);
+struct alignas(4) X {
+  X(const X &);
+  X(X &&);
 };
 
 #define L(A, B, C) void l##A() {    \
   auto t = []<class T = X>() -> C { \
     T t;                            \
     return B;                       \
   }();                              \
 }

 void b_attr() {
   auto t = []<class T = X>() {
     return ^X() [[clang::vectorcall]] {
       T t;
       return t;
     };
   }()();
 }
+
+namespace test_alignas {
+
+template <int A> X t1() {
+  X a [[gnu::aligned(A)]];
+  return a;
+}
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t1ILi1EEE1Xv
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  ret void
+template X t1<1>();
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t1ILi4EEE1Xv
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  ret void
+template X t1<4>();
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t1ILi8EEE1Xv
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
+// CHECK-NEXT:  call void @llvm.lifetime.end
+template X t1<8>();
+
+template <int A> X t2() {
+  X a [[gnu::aligned(1)]] [[gnu::aligned(A)]] [[gnu::aligned(2)]];
+  return a;
+}
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t2ILi1EEE1Xv
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  ret void
+template X t2<1>();
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t2ILi4EEE1Xv
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  ret void
+template X t2<4>();
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t2ILi8EEE1Xv
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
+// CHECK-NEXT:  call void @llvm.lifetime.end
+template X t2<8>();
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t3Ev
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  ret void
+X t3() {
+  X a [[gnu::aligned(1)]];
+  return a;
+}
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t4Ev
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
+// CHECK-NEXT:  call void @llvm.lifetime.end
+X t4() {
+  X a [[gnu::aligned(8)]];
+  return a;
+}
+
+// CHECK-LABEL: define{{.*}} void @_ZN12test_alignas2t5Ev
+// CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
+// CHECK-NEXT:  call void @llvm.lifetime.end
+X t5() {
+  X a [[gnu::aligned(1)]] [[gnu::aligned(8)]];
+  return a;
+}
+
+} // namespace test_alignas