[cfe-commits] [PATCH] Validate C99 and C++03's set of UCNs

Jordan Rose jordan_rose at apple.com
Fri Jan 25 10:53:28 PST 2013


  Will post a revised version soon.


================
Comment at: lib/Lex/CharSets.inc:379-381
@@ +378,5 @@
+
+// C99 6.4.2.1p3: The initial character [of an identifier] shall not be a
+// universal character name designating a digit.
+// C99 Annex D defines these characters as "Digits".
+static const UnicodeCharRange C99DisallowedInitialIDChars[] = {
----------------
Richard Smith wrote:
> How sure are you about this? It might also be referring to the term /digit/ defined in 6.4.2.1/1 as 'one of 0 1 2 3 4 5 6 7 8 9', and that would be more consistent with C11's rules, in which all of these characters may start an identifier, and only combining characters are ruled out at the start of an identifier.
> 
> By the other interpretation of this rule, it would be redundant given that such UCNs would be ill-formed by 6.4.3/2, but I'm really not sure about the intent here.
Interesting. I had assumed this interpretation simply because of the redundancy. http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1518.htm seems to suggest that while my original interpretation is most likely, it's not well-specified.

================
Comment at: lib/Lex/Lexer.cpp:1524-1525
@@ -1554,1 +1523,4 @@
+
+template <size_t N>
+static bool isCharInSet(uint32_t c, const UnicodeCharRange (&Ranges)[N]) {
   unsigned LowPoint = 0;
----------------
Richard Smith wrote:
> Hmm, interesting, did you template this on the array size for convenience, or in the hope that the compiler will unroll the loop? If the latter, did it work? (And if so, did we manage to propagate the array into the unrolled loop, or is it still being passed as a parameter?)
Convenience, and the whole thing will change because Joerg pointed out that it's worth validating the arrays.

I did check, and it looks like even at -O3 the loop isn't unrolled (at least not in IR). Did get inlined, though.


http://llvm-reviews.chandlerc.com/D327



More information about the cfe-commits mailing list