On May 7, 2014, at 4:25 PM, Tyler Nowicki <[email protected]> wrote:
> Hi Ben, > > Thanks for the review. I have made most of the changes, but I have a few > questions. > >>> I think it makes sense because c++11 attributes, for example [[loop >>> vectorize(4)]], would be verified at this point too. >> >> Why is that? Shouldn’t they both be dealt with in Sema? I don’t like the >> idea that I wouldn’t get these semantic diagnostics with -fsyntax-only. > > Are you thinking this should be put in AnalysisBasedWarnings or something? > Can you be more specific about where in Sema and what calls it? If you follow Richard’s suggestion to add an ActOnPragmaLoopHint you would check it from somewhere under there, which would be called from ParsePragmaLoopHint. You should be able to use Expr::isIntegerConstantExpr() to evaluate the constant expressions. > > >>> Index: test/Parser/pragma-loop2.cpp >>> =================================================================== >>> --- test/Parser/pragma-loop2.cpp (revision 0) >>> +++ test/Parser/pragma-loop2.cpp (working copy) >>> @@ -0,0 +1,50 @@ >>> +// RUN: not %clang %s 2>&1 | FileCheck %s >> >> This test just highlights why this should be done in Sema if at all >> possible… Also, you probably don’t need the driver here. > > What is the driver? ‘clang' is the driver as opposed to ‘clang -cc1’, which is the frontend. The driver builds the pipeline of compilations jobs (e.g. compile, link, etc) and delegates those jobs to various tools (clang -cc1, ld, etc.). The driver does strictly more than your tests need. The driver is also much more host-dependent than the frontend, so we prefer -cc1 in tests so that they depend less on the machine they are running on. Ben > > > I will provide another patch when responding to Aaron’s review. > > Tyler _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
