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.

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

Jump to bottom

[clang] Availability attribute only triggers on the second use of a variable #61815

Closed
ldionne opened this issue Mar 29, 2023 · 7 comments
Closed

[clang] Availability attribute only triggers on the second use of a variable #61815

ldionne opened this issue Mar 29, 2023 · 7 comments
Assignees
@shafik
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party

Comments

@ldionne
Copy link
Member

ldionne commented Mar 29, 2023 •
edited by VoltrexKeyva
Loading 

cat <<EOF | clang++ -xc++ - -fsyntax-only
template <class _ValueType = int>
class __attribute__((unavailable)) polymorphic_allocator { };

void f() {
    polymorphic_allocator<void> a;
    polymorphic_allocator<void> b;
}
EOF

This fails with:

<stdin>:6:5: error: 'polymorphic_allocator<void>' is unavailable
    polymorphic_allocator<void> b;
    ^
<stdin>:2:36: note: 'polymorphic_allocator<void>' has been explicitly marked unavailable here
class __attribute__((unavailable)) polymorphic_allocator { };
                                   ^
1 error generated.

In reality, it should have failed on the first usage of polymorphic_allocator<void>, which is when declaring a.

Also on Godbolt: https://godbolt.org/z/aEsfjcdT1
@ldionne ldionne added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Mar 29, 2023
@llvmbot
Copy link
Member

llvmbot commented Mar 29, 2023

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Mar 30, 2023 •
edited
Loading

It looks like the first time through parsing f() when we parse the declaration of a we don't have the complete type for the ClassTemplateSpecializationDecl so the attributes are not available but the next time around we do.

I don't see any obvious place to fix this in the parsing but I am not super familiar with this part but I was able to modify Decl::getAvailability(....) to fix this, I noticed we had a carve out for FunctionTemplateDecl so I added one for ClassTemplateSpecializationDecl like so:

if (auto *FTD = dyn_cast<FunctionTemplateDecl>(this))
    return FTD->getTemplatedDecl()->getAvailability(Message, EnclosingVersion,
                                                    RealizedPlatform);
  else if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(this))
    return CTSD->getSpecializedTemplate()->getTemplatedDecl()->getAvailability(Message, EnclosingVersion,
                                                    RealizedPlatform);

I don't know if this is the right way or if this indicates a parsing design issue that was worked around before and I am just adding another work-around.

@AaronBallman wdyt?

@shafik
Copy link
Collaborator

shafik commented Mar 30, 2023

CC @zygoloid who may also know the answer to this as well

@zygoloid
Copy link
Collaborator

zygoloid commented Mar 30, 2023 •
edited
Loading

The problem here is that this diagnostic is produced before we require polymorphic_allocator<void> to be a complete type -- and indeed in similar examples we never require it to be a complete type. For example:

extern polymorphic_allocator<void> x;

... should presumably warn, but doesn't instantiate the class template definition.

Most attributes are only instantiated with the class template definition, rather than being instantiated with the declaration. There's a flag on the attribute definition in Attr.td to indicate that the attribute should instead be instantiated with the declaration. Surprisingly, this flag is called MeaningfulToClassTemplateDefinition and its doc comment is backwards too... Setting that to 1 for the Unavailable attr definition should fix this.

@shafik
Copy link
Collaborator

shafik commented Mar 30, 2023

Yup, that does it, adding let MeaningfulToClassTemplateDefinition = 1; fixes this issue.

@shafik shafik self-assigned this Mar 30, 2023
@AaronBallman
Copy link
Collaborator

Surprisingly, this flag is called MeaningfulToClassTemplateDefinition and its doc comment is backwards too... Setting that to 1 for the Unavailable attr definition should fix this.

Whoops, looks like we got the naming wrong there -- that came in through https://reviews.llvm.org/D31245

@shafik shafik added the confirmed Verified by a second party label Apr 4, 2023
@shafik
Copy link
Collaborator

shafik commented Apr 6, 2023

PR: https://reviews.llvm.org/D147495

@shafik shafik closed this as completed in b206cde Apr 6, 2023
gysit pushed a commit to nextsilicon/llvm-project that referenced this issue Apr 27, 2023
@shafik @gysit
[Clang][Attributes] Add MeaningfulToClassTemplateDefinition to unavai…
489deb3 
…lable attribute

There may be cases in which we want to diagnose a type as unavailable but it may
not be complete at the time. Setting MeaningfulToClassTemplateDefinition fixes
this issue.

This fixes: llvm#61815

Differential Revision: https://reviews.llvm.org/D147495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shafik shafik

Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@zygoloid @ldionne @AaronBallman @shafik @EugeneZelenko @llvmbot