erichkeane added a comment.

I have one 'nit of preference', otherwise I think I don't want to +1 this 
without giving people a few days to take a look.  Based on my looks through the 
surrounding code, this _LOOKS_ right enough as far as I can tell, but I still 
want to give others a few days.



================
Comment at: clang/lib/Lex/PPDirectives.cpp:621
+        if (CondInfo.FoundElse)
+            Diag(Tok, diag::pp_err_elif_after_else) << (IsElifDef ? 1 : 2);
+
----------------
As you know, I'm a giant fan of using enums for cases like this, so it would be 
my preference to have something like that.  Sadly it looks like it would have 
to be file-level here instead of functionlevel like is most convenient.


================
Comment at: clang/test/Preprocessor/elifdef.c:71
+
+/* expected-error@+3 {{#elifdef after #else}} */
+#ifdef FOO
----------------
Are there any tests you could do to make sure this 'works'?  That is:

#define BAR
// expected-error@+3 {{"AN ERROR!"}}
#ifdef FOO
#elifdef BAR
#error "AN ERROR!"
#endif

AND

// no error expected here!
#ifdef FOO
#elifndef BAR
#error "AN ERROR!"
#endif

And perhaps...

// expected-error@+4 {{"AN ERROR AGAIN!"}}
#ifdef FOO
#elifdef BAR
#else
#error "AN ERROR AGAIN!"
#endif 




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

https://reviews.llvm.org/D101192

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to