[PATCH] [MSVC] Improved __noop support (https://llvm.org/bugs/show_bug.cgi?id=14081)

Richard Smith richard at metafoo.co.uk
Tue Apr 14 16:04:31 PDT 2015


================
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);
+            }
+          }
----------------
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?

================
Comment at: lib/Parse/ParseExpr.cpp:1466-1467
@@ -1457,1 +1465,4 @@
+          }
+          Sema::InsideNoopRAII SubstIndex(Actions,
+                                          BuiltinId == Builtin::BI__noop);
           if (ParseExpressionList(ArgExprs, CommaLocs, [&] {
----------------
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).

http://reviews.llvm.org/D9000

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list