Skip to content

[Sema] Don't call isNonConstantStorage on incomplete variable types #161590

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

[Sema] Don't call isNonConstantStorage on incomplete variable types #161590

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a46d411
[Sema] Don't call isNonConstantStorage on incomplete variable types
ahatanak Oct 1, 2025
1ce0335
Don't emit the diagnostic unless the variable is instantiated
ahatanak Oct 2, 2025
f74f86c
Add test case
ahatanak Oct 9, 2025
0bf70a5
Add a release note entry
ahatanak Oct 30, 2025
c37a82d
Merge branch 'main' into attr-section-incomplete-type
ahatanak Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 50 additions & 45 deletions clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14918,52 +14918,57 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
}

// Apply section attributes and pragmas to global variables.
if (GlobalStorage && var->isThisDeclarationADefinition() &&
!inTemplateInstantiation()) {
PragmaStack<StringLiteral *> *Stack = nullptr;
int SectionFlags = ASTContext::PSF_Read;
bool MSVCEnv =
Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment();
std::optional<QualType::NonConstantStorageReason> Reason;
if (HasConstInit &&
!(Reason = var->getType().isNonConstantStorage(Context, true, false))) {
Stack = &ConstSegStack;
} else {
SectionFlags |= ASTContext::PSF_Write;
Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack;
}
if (const SectionAttr *SA = var->getAttr<SectionAttr>()) {
if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec)
[&]() {
if (GlobalStorage && var->isThisDeclarationADefinition() &&
!inTemplateInstantiation()) {
PragmaStack<StringLiteral *> *Stack = nullptr;
int SectionFlags = ASTContext::PSF_Read;
bool MSVCEnv =
Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment();
std::optional<QualType::NonConstantStorageReason> Reason;
if (HasConstInit && var->getType()->isIncompleteType())
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't seem like it's quite the right thing to check; we shouldn't be be trying to compute the section at all for a definition inside a templated class/function, no matter what the type is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we stop computing the section for definitions inside templated classes/functions altogether, clang will stop emitting error 'm' causes a section type conflict with 'v1' when the following code is compiled. Is that okay?

template <class T>
struct C {
  __attribute__((section("non_trivial_ctor")))
  static constexpr int m{123};
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems fine. It's not actually causing a section conflict unless you instantiate it.

if (HasConstInit && !(Reason = var->getType().isNonConstantStorage(
Context, true, false))) {
Stack = &ConstSegStack;
} else {
SectionFlags |= ASTContext::PSF_Write;
Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack;
}
if (const SectionAttr *SA = var->getAttr<SectionAttr>()) {
if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec)
SectionFlags |= ASTContext::PSF_Implicit;
UnifySection(SA->getName(), SectionFlags, var);
} else if (Stack->CurrentValue) {
if (Stack != &ConstSegStack && MSVCEnv &&
ConstSegStack.CurrentValue != ConstSegStack.DefaultValue &&
var->getType().isConstQualified()) {
assert((!Reason || Reason != QualType::NonConstantStorageReason::
NonConstNonReferenceType) &&
"This case should've already been handled elsewhere");
Diag(var->getLocation(), diag::warn_section_msvc_compat)
<< var << ConstSegStack.CurrentValue
<< (int)(!HasConstInit
? QualType::NonConstantStorageReason::NonTrivialCtor
: *Reason);
}
SectionFlags |= ASTContext::PSF_Implicit;
UnifySection(SA->getName(), SectionFlags, var);
} else if (Stack->CurrentValue) {
if (Stack != &ConstSegStack && MSVCEnv &&
ConstSegStack.CurrentValue != ConstSegStack.DefaultValue &&
var->getType().isConstQualified()) {
assert((!Reason || Reason != QualType::NonConstantStorageReason::
NonConstNonReferenceType) &&
"This case should've already been handled elsewhere");
Diag(var->getLocation(), diag::warn_section_msvc_compat)
<< var << ConstSegStack.CurrentValue << (int)(!HasConstInit
? QualType::NonConstantStorageReason::NonTrivialCtor
: *Reason);
}
SectionFlags |= ASTContext::PSF_Implicit;
auto SectionName = Stack->CurrentValue->getString();
var->addAttr(SectionAttr::CreateImplicit(Context, SectionName,
Stack->CurrentPragmaLocation,
SectionAttr::Declspec_allocate));
if (UnifySection(SectionName, SectionFlags, var))
var->dropAttr<SectionAttr>();
}

// Apply the init_seg attribute if this has an initializer. If the
// initializer turns out to not be dynamic, we'll end up ignoring this
// attribute.
if (CurInitSeg && var->getInit())
var->addAttr(InitSegAttr::CreateImplicit(Context, CurInitSeg->getString(),
CurInitSegLoc));
}
auto SectionName = Stack->CurrentValue->getString();
var->addAttr(SectionAttr::CreateImplicit(
Context, SectionName, Stack->CurrentPragmaLocation,
SectionAttr::Declspec_allocate));
if (UnifySection(SectionName, SectionFlags, var))
var->dropAttr<SectionAttr>();
}

// Apply the init_seg attribute if this has an initializer. If the
// initializer turns out to not be dynamic, we'll end up ignoring this
// attribute.
if (CurInitSeg && var->getInit())
var->addAttr(InitSegAttr::CreateImplicit(
Context, CurInitSeg->getString(), CurInitSegLoc));
}
}();

// All the following checks are C++ only.
if (!getLangOpts().CPlusPlus) {
Expand Down
8 changes: 8 additions & 0 deletions clang/test/SemaCXX/attr-section.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,11 @@ __attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note {{dec
extern const t1 v2;
__attribute__((section("non_trivial_ctor"))) const t1 v2{3}; // expected-error {{'v2' causes a section type conflict with 'v1'}}
} // namespace non_trivial_ctor

namespace incomplete_type {
template <class T>
struct A {
struct B;
static constexpr B b{nullptr};
};
}