This is a kind reminder to review the patch.
================
Comment at: lib/Parse/ParseExpr.cpp:1459-1464
@@ +1458,8 @@
+ if (LHS.isUsable()) {
+ Expr *Fn = LHS.get()->IgnoreParens();
+ if (DeclRefExpr *DRE = dyn_cast_or_null<DeclRefExpr>(Fn)) {
+ FunctionDecl *FnDecl =
+ dyn_cast_or_null<FunctionDecl>(DRE->getDecl());
+ BuiltinId = (FnDecl ? FnDecl->getBuiltinID() : 0);
+ }
+ }
----------------
rsmith wrote:
> Code in Parse shouldn't be inspecting the AST like this. If you really need
> different parse rules for the argument of __noop, maybe make it a keyword
> rather than a builtin?
Firstly I tried to make __noop as a separate keyword however __noop has a
builtin behavior in 95% cases. For example __noop will be a normal variable
after "int __noop;" declaration. I suppose that it will be inefficient to
detect all these cases for a new key-word.
================
Comment at: lib/Parse/ParseExpr.cpp:1466-1467
@@ -1457,1 +1465,4 @@
+ }
+ Sema::InsideNoopRAII SubstIndex(Actions,
+ BuiltinId == Builtin::BI__noop);
if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
----------------
rsmith wrote:
> Can you enter an unevaluated context here rather than inventing a new sort of
> context for this case? Global state in Sema doesn't seem like the right way
> to handle this -- if you trigger an instantiation from within this context,
> you should not instantiate the template in noop mode (it might end up being
> used outside noop mode too).
Entering unevaluated context doesn’t prevent from template instantiation here.
Global state points that we are inside __noop parsing now.
================
Comment at: lib/Sema/SemaExpr.cpp:13055
@@ +13054,3 @@
+ // This behaviour is the same as Microsoft compiler behaviour.
+ if (!InsideNoop || !isa<FunctionDecl>(E->getDecl()))
+ MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse);
----------------
rnk wrote:
> rnk wrote:
> > Why do we need to mark non-FunctionDecls referenced? Surely they can
> > trigger template instantiation of other function decls.
> Can this just be `if (InsideNoop) return;` at the beginning of the function?
Non-FunctionDecls are marked as referenced because this prevents warnings about
unused variables
================
Comment at: test/Sema/builtin-ms-noop-errors.cpp:16
@@ +15,3 @@
+ int var1, var2, var3, var4, var5, var6, var7, var8;
+ ((__noop))(function2<void>()); // expected-note{{'__noop' is a builtin with
type 'int ()'}}
+ __noop(Z(), Z(), (function2<int>()), Z1(function2<int>()));
----------------
chatur01 wrote:
> I find checking this note weird. I know `__noop` is defined as taking `...`
> from the Builtins.def file, but `ASTContext::GetBuiltinType` decides to
> ignore it and generate the empty argument list instead.
This patch doesn’t influence on this. I simply added this case (error + note)
in a test. Deleted them.
http://reviews.llvm.org/D9000
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
