[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/DeclSpec.cpp:220
+
+  I.Fun.QualAttrFactory         = nullptr;
+
----------------
Anastasia wrote:
> rjmccall wrote:
> > Spacing?
> I am keeping  the old formatting from the lines 195-204. But obviously it doesn't match with the current clang-format rules. Do you prefer that I reformat the whole block or just this line?
Oh, I see.  Could you hoist the null-initialization of these two fields above this block, then?  They'll look more consistent, and it'll make it clearer that you're establishing the basic invariants on these fields before modifying them.


================
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:
> > 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?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55948/new/

https://reviews.llvm.org/D55948