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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits