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 3 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 all commits
Commits
Show all changes
3 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
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
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14919,7 +14919,7 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {

// Apply section attributes and pragmas to global variables.
if (GlobalStorage && var->isThisDeclarationADefinition() &&
!inTemplateInstantiation()) {
!var->getDeclContext()->isDependentContext()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you try !type->isDependentType() - we do that elsewhere

Copy link
Collaborator

Choose a reason for hiding this comment

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

To fill in some details, it is more consistent and it is cheaper since isDependentContext() looks at all the parent contexts recursively.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Checking JUST the actual var decl is actually going to be a 'smaller' thing, but I'm not sure it is actually going to work? @ahatanak : please try it though, it is quite a bit easier.

The difference here is that if we switch to ONLY a dependent type (instead of a dependent decl), we end up in a situation where we might try to do this for a non-dependently typed variable in a dependent context. So I don't think that'll fix it?

Consider the example:

template <class T>
struct A {
  struct B;
  static constexpr B b{nullptr}; // This used to crash.
};

IN this case, B is a dependent type, since it is inside the struct (so it is A<T>::B. However consider this slight modification (please add this test!).

  struct B;
template <class T>
struct A {

  static constexpr B b{nullptr}; // This used to crash.
};

Note I moved B out of the template. Now b is still going to go down this path (I think?) but is no longer a dependent type, despite being a dependent context. I think this is wrong?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, clang computes the section and emits the diagnostic for b without instantiation if !type->isDependentType() is called instead of !var->getDeclContext()->isDependentContext().

struct B1 { void *p; };
template <class T>
struct A1 {
  __attribute__((section("non_trivial_ctor")))
  static constexpr B1 b{nullptr};
};

// auto *p = &A1<int>::b;

PragmaStack<StringLiteral *> *Stack = nullptr;
int SectionFlags = ASTContext::PSF_Read;
bool MSVCEnv =
Expand Down
25 changes: 24 additions & 1 deletion clang/test/SemaCXX/attr-section.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,30 @@ struct t1 {
constexpr t1(int) { }
};
extern const t1 v1;
__attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note {{declared here}}
__attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note 2 {{declared here}}
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 dependent_context {
template <class T>
struct A {
struct B;
static constexpr B b{nullptr}; // This used to crash.
};

struct B1 { void *p; };
template <class T>
struct A1 {
__attribute__((section("non_trivial_ctor")))
static constexpr B1 b{nullptr}; // no diagnostic expected
};

template <class T>
struct C {
__attribute__((section("non_trivial_ctor")))
static constexpr int m{123}; // expected-error {{'m' causes a section type conflict with 'v1'}}
};

auto *p = &C<int>::m; // expected-note {{in instantiation of static data member 'dependent_context::C<int>::m' requested here}}
}