check type attribute arguments as well

Author: tbaederr

clang/include/clang/Sema/Sema.h

@@ -13868,6 +13868,11 @@ class Sema final {
  SourceLocation BuiltinLoc,
  SourceLocation RParenLoc);
 
+ void checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL,
+ SmallVectorImpl<Expr *> &Args,
+ unsigned Sidx = 0,
+ bool ParamIdxOk = false);
+
 private:
  bool SemaBuiltinPrefetch(CallExpr *TheCall);
  bool SemaBuiltinAllocaWithAlign(CallExpr *TheCall);

clang/lib/AST/TypePrinter.cpp

@@ -1894,6 +1894,8 @@ void TypePrinter::printAttributedAfter(const AttributedType *T,
  case attr::ArmMveStrictPolymorphism:
  OS << "__clang_arm_mve_strict_polymorphism";
  break;
+ case attr::RequiresCapability:
+ OS << "requires_capability(blah)";
  }
  OS << "))";
 }

clang/lib/Sema/SemaDeclAttr.cpp

@@ -600,12 +600,10 @@ static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
 /// \param Sidx The attribute argument index to start checking with.
 /// \param ParamIdxOk Whether an argument can be indexing into a function
 /// parameter list.
-static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
- const ParsedAttr &AL,
- SmallVectorImpl<Expr *> &Args,
- unsigned Sidx = 0,
- bool ParamIdxOk = false) {
- if (Sidx == AL.getNumArgs()) {
+void Sema::checkAttrArgsAreCapabilityObjs(Decl *D, const ParsedAttr &AL,
+ SmallVectorImpl<Expr *> &Args,
+ unsigned Sidx, bool ParamIdxOk) {
+ if (D && Sidx == AL.getNumArgs()) {
  // If we don't have any capability arguments, the attribute implicitly
  // refers to 'this'. So we need to make sure that 'this' exists, i.e. we're
  // a non-static method, and that the class is a (scoped) capability.
@@ -615,11 +613,10 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
  // FIXME -- need to check this again on template instantiation
  if (!checkRecordDeclForAttr<CapabilityAttr>(RD) &&
  !checkRecordDeclForAttr<ScopedLockableAttr>(RD))
- S.Diag(AL.getLoc(),
- diag::warn_thread_attribute_not_on_capability_member)
+ Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_capability_member)
  << AL << MD->getParent();
  } else {
- S.Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)
+ Diag(AL.getLoc(), diag::warn_thread_attribute_not_on_non_static_member)
  << AL;
  }
  }
@@ -644,7 +641,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
 
  // We allow constant strings to be used as a placeholder for expressions
  // that are not valid C++ syntax, but warn that they are ignored.
- S.Diag(AL.getLoc(), diag::warn_thread_attribute_ignored) << AL;
+ Diag(AL.getLoc(), diag::warn_thread_attribute_ignored) << AL;
  Args.push_back(ArgExp);
  continue;
  }
@@ -663,7 +660,7 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
  const RecordType *RT = getRecordType(ArgTy);
 
  // Now check if we index into a record type function param.
- if(!RT && ParamIdxOk) {
+ if (D && !RT && ParamIdxOk) {
  const auto *FD = dyn_cast<FunctionDecl>(D);
  const auto *IL = dyn_cast<IntegerLiteral>(ArgExp);
  if(FD && IL) {
@@ -672,8 +669,8 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
  uint64_t ParamIdxFromOne = ArgValue.getZExtValue();
  uint64_t ParamIdxFromZero = ParamIdxFromOne - 1;
  if (!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) {
- S.Diag(AL.getLoc(),
-  diag::err_attribute_argument_out_of_bounds_extra_info)
+ Diag(AL.getLoc(),
+ diag::err_attribute_argument_out_of_bounds_extra_info)
  << AL << Idx + 1 << NumParams;
  continue;
  }
@@ -685,8 +682,8 @@ static void checkAttrArgsAreCapabilityObjs(Sema &S, Decl *D,
  // expression have capabilities. This allows for writing C code where the
  // capability may be on the type, and the expression is a capability
  // boolean logic expression. Eg) requires_capability(A || B && !C)
- if (!typeHasCapability(S, ArgTy) && !isCapabilityExpr(S, ArgExp))
- S.Diag(AL.getLoc(), diag::warn_thread_attribute_argument_not_lockable)
+ if (!typeHasCapability(*this, ArgTy) && !isCapabilityExpr(*this, ArgExp))
+ Diag(AL.getLoc(), diag::warn_thread_attribute_argument_not_lockable)
  << AL << ArgTy;
 
  Args.push_back(ArgExp);
@@ -708,7 +705,7 @@ static bool checkGuardedByAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
  Expr *&Arg) {
  SmallVector<Expr *, 1> Args;
  // check that all arguments are lockable objects
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
  unsigned Size = Args.size();
  if (Size != 1)
  return false;
@@ -750,7 +747,7 @@ static bool checkAcquireOrderAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
  }
 
  // Check that all arguments are lockable objects.
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
  if (Args.empty())
  return false;
 
@@ -781,7 +778,7 @@ static bool checkLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
  SmallVectorImpl<Expr *> &Args) {
  // zero or more arguments ok
  // check that all arguments are lockable objects
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, /*ParamIdxOk=*/true);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 0, /*ParamIdxOk=*/true);
 
  return true;
 }
@@ -883,7 +880,7 @@ static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL,
  }
 
  // check that all arguments are lockable objects
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 1);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 1);
 
  return true;
 }
@@ -911,7 +908,7 @@ static void handleExclusiveTrylockFunctionAttr(Sema &S, Decl *D,
 static void handleLockReturnedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
  // check that the argument is lockable object
  SmallVector<Expr*, 1> Args;
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
  unsigned Size = Args.size();
  if (Size == 0)
  return;
@@ -925,7 +922,7 @@ static void handleLocksExcludedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
  // check that all arguments are lockable objects
  SmallVector<Expr*, 1> Args;
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
  unsigned Size = Args.size();
  if (Size == 0)
  return;
@@ -8176,7 +8173,7 @@ static void handleReleaseCapabilityAttr(Sema &S, Decl *D,
  const ParsedAttr &AL) {
  // Check that all arguments are lockable objects.
  SmallVector<Expr *, 1> Args;
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args, 0, true);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args, 0, true);
 
  D->addAttr(::new (S.Context) ReleaseCapabilityAttr(S.Context, AL, Args.data(),
  Args.size()));
@@ -8189,7 +8186,7 @@ static void handleRequiresCapabilityAttr(Sema &S, Decl *D,
 
  // check that all arguments are lockable objects
  SmallVector<Expr*, 1> Args;
- checkAttrArgsAreCapabilityObjs(S, D, AL, Args);
+ S.checkAttrArgsAreCapabilityObjs(D, AL, Args);
  if (Args.empty())
  return;
 

clang/lib/Sema/SemaType.cpp

@@ -8652,15 +8652,9 @@ static void HandleRequiresCapabilityAttr(TypeProcessingState &State,
  return;
  }
 
- // FIXME: We need to sanity check the arguments here I think? Like we do in
- // SemaDeclAtr.cpp.
-
  llvm::SmallVector<Expr *, 4> Args;
  Args.reserve(PA.getNumArgs() - 1);
- for (unsigned Idx = 1; Idx < PA.getNumArgs(); Idx++) {
- assert(!PA.isArgIdent(Idx));
- Args.push_back(PA.getArgAsExpr(Idx));
- }
+ State.getSema().checkAttrArgsAreCapabilityObjs(/*Decl=*/nullptr, PA, Args);
 
  auto *RCAttr =
  RequiresCapabilityAttr::Create(S.Context, Args.data(), Args.size(), PA);