rjmccall added inline comments.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:8175
DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
- if (FTI.TypeQuals != 0) {
- if (FTI.TypeQuals & Qualifiers::Const)
- Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
- << "const" << SourceRange(D.getIdentifierLoc());
- if (FTI.TypeQuals & Qualifiers::Volatile)
- Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
- << "volatile" << SourceRange(D.getIdentifierLoc());
- if (FTI.TypeQuals & Qualifiers::Restrict)
- Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_constructor)
- << "restrict" << SourceRange(D.getIdentifierLoc());
+ if (FTI.MethodQualifiers && FTI.MethodQualifiers->getTypeQualifiers() != 0) {
+ auto DiagQual = [&](DeclSpec::TQ TypeQual, StringRef QualName,
----------------
Anastasia wrote:
> rjmccall wrote:
> > Anastasia wrote:
> > > rjmccall wrote:
> > > > Anastasia wrote:
> > > > > rjmccall wrote:
> > > > > > I think you should add a `hasMethodQualifiers` method to FTI that
> > > > > > does this check. Note that it needs to check for attributes, too,
> > > > > > and I think you need to figure out some way to generalize
> > > > > > `forEachCVRUQual` to cover those.
> > > > > Are there any attributes I should handle currently?
> > > > >
> > > > > Also are you suggesting to add another `forEach...` method or extend
> > > > > existing? If the latter, I might not be able to use it in all places
> > > > > I use it now.
> > > > Adding another method might be easier. How many clients actually use
> > > > the TQ?
> > > In **DeclSpec.cpp** I definitely need just TQ. I am not sure about
> > > **SemaType.cpp**. All other places (3x) I guess should be possible to
> > > generalize. Although I am not very clear if I should be checking all
> > > attr. It might be a bit exhaustive since the use cases are for the
> > > function?
> > >
> > > Perhaps, I could add an extra helper `forEachQualifier` that can call
> > > `forEachCVRUQual` and then I could add a FIXME to complete the rest. We
> > > can extend it as we discover what's missing. For example I will add
> > > address spaces there in my next patch. Would this make sense?
> > >
> > > As for `hasMethodQualifiers` just to be clear I would need to check for
> > > all qualifiers including reference qualifier, attributes, etc?
> > That seems like a reasonable short-term plan. Maybe there needs to be some
> > way to describe an individual qualifier; we can hash that out in a separate
> > patch.
> >
> > > As for `hasMethodQualifiers` just to be clear I would need to check for
> > > all qualifiers including reference qualifier, attributes, etc?
> >
> > Maybe, although at least one of the cases below wants to check for
> > ref-qualifiers separately. Maybe it should be `hasMethodTypeQualifiers`,
> > and it implies that `MethodQualifiers->forEachQualifier` will invoke the
> > callback at least once.
> I think it should be sufficient to check that `MethodQualifiers` exist
> because we only create it if we have either a type qual or any attribute.
Hmm. I think we can remove qualifiers, and we do sometimes (at least in
recovery situations, and possibly in others), so it's better not to rely on
that. But I think for attributes you can just check whether there are any
attributes rather than carefully checking whether any attribute is a type
qualifier.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55948/new/
https://reviews.llvm.org/D55948
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
