[PATCH] [PATCH] [CONCEPTS] Parsing of concept keyword

[Richard Smith]

================
Comment at: lib/Sema/DeclSpec.cpp:899
@@ +898,3 @@
+  if (Concept_specified) {
+    DiagID = diag::warn_duplicate_declspec;
+    PrevSpec = "concept";
----------------
nwilson wrote:
> rsmith wrote:
> > Why just a warning? [dcl.spec]/2 seems to apply to `concept` just like any other //decl-specifier//.
> Wouldn't this be similar to constexpr (above)? Do you think it should error out?
Yes, it should. (We don't yet implement DR1830 so the pre-existing cases aren't caught yet.)

================
Comment at: test/Parser/cxx-concepts-value-function.cpp:11
@@ +10,3 @@
+
+#ifdef DIAG
+
----------------
nwilson wrote:
> rsmith wrote:
> > hubert.reinterpretcast wrote:
> > > nwilson wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > -DDIAG=0 would still trigger the #ifdef DIAG here (which explains why the test file might pass with expected-no-diagnostics).
> > > > > I meant //without// `expected-no-diagnostics` somewhere.
> > > > Hmm, yeah, I missed that and thought it was working. Is there a better way to use/suppress the diagnostic cases?
> > > `#if DIAG` instead of `#ifdef` would work. I guess `expected-no-diagnostics` could be in the `#else` for that.
> > Just run the test once, and remove the `#ifdef`.
> I'll submit a patch for this shortly so you guys can see if this explanation doesn't make sense. But, how about if I use the directives likes this:
> 
> #if DIAG == 0
> // RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s -DDIAG=0
> // expected-no-diagnostics
> 
> *** test cases ***
> 
> #elif DIAG == 1
> // RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ -verify %s -DDIAG=1
> 
> *** test cases ***
> 
> #endif
> 
> 
What are you trying to achieve by running the test twice?

================
Comment at: test/Parser/cxx-concepts-value-function.cpp:15
@@ +14,3 @@
+
+template< concept T > concept bool D2 { return true; } // expected-error {{unknown type name 'T'}} expected-error {{expected expression}} expected-error {{expected ';' at end of declaration}}
+
----------------
nwilson wrote:
> rsmith wrote:
> > Which of the various errors on this line are you trying to test here?
> All of those diagnostics are generated, so I guess all of them.
> 
> Is there a better way to do it? Should I only handle one?
> 
It'd be clearer to have a separate declaration for each different case you're trying to test, so that someone who causes the test to fail (say, by changing the parser's error recovery) can understand whether they've actually broken something.

http://reviews.llvm.org/D10528

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