[PATCH] D94092: [Clang] Remove unnecessary Attr.isArgIdent checks.

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 5 08:44:16 PST 2021


erichkeane added a comment.

I'm not sure how well Attr.td's constraints are enforced on type attributes, as these often happen before parsing is completely done.  I'd imagine this code was put into place at least the 1st time for good reason, but I'm curious as to why we wouldn't have tests that cover that (or, as you assert, it could simply be that this is simply dead code).

I'm generally OK with this (the asserts are unnecessary), but would like @aaron.ballman to double check my expectations here.



================
Comment at: clang/lib/Sema/SemaType.cpp:7661
 
-  Expr *SizeExpr;
-  // Special case where the argument is a template id.
-  if (Attr.isArgIdent(0)) {
-    CXXScopeSpec SS;
-    SourceLocation TemplateKWLoc;
-    UnqualifiedId Id;
-    Id.setIdentifier(Attr.getArgAsIdent(0)->Ident, Attr.getLoc());
-
-    ExprResult Size = S.ActOnIdExpression(S.getCurScope(), SS, TemplateKWLoc,
-                                          Id, /*HasTrailingLParen=*/false,
-                                          /*IsAddressOfOperand=*/false);
-
-    if (Size.isInvalid())
-      return;
-    SizeExpr = Size.get();
-  } else {
-    SizeExpr = Attr.getArgAsExpr(0);
-  }
+  assert(Attr.isArgExpr(0) && "Argument to should be an expression");
+  Expr *SizeExpr = Attr.getArgAsExpr(0);
----------------
I think the asserts aren't necessary, isArgExpr does a ArgsUnion::get<Expr*>, which already asserts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94092



More information about the cfe-commits mailing list