[cfe-commits] r59803 - in /cfe/trunk:

[Chris Lattner]

On Nov 21, 2008, at 2:43 PM, Sebastian Redl wrote:

> Chris Lattner wrote:
>> On Nov 21, 2008, at 4:23 AM, Sebastian Redl wrote:
>>> I was planning to implement a modifier that works like this:
>>> "We have %0 %c0{1:mouse|:mice}"
>>> i.e. a simple case statement. ("case parameter 0 in") The modifer  
>>> picks an
>>> integral argument and executes the case. It could be more  
>>> complicated for
>>> languages that, say, have different forms for 0, 1, 2, and more  
>>> cases:
>>> "%c0{0:something|1:somethingelse|2:yetanotherthing|:finaloption}"
>>
>> Sure, this works for me.  A few requests: please use a longer  
>> modifier name than "c" (I don't have any specific suggestions  
>> though).  My intent is for tricky things to use longer names (like  
>> 'select') and very few but common things to use short names like  
>> 's'.   Second, please consider supporting ranges:  
>> %whatever{3-7:blah}, so we can handle Polish:
>> http://www.gnu.org/software/automake/manual/gettext/Plural-forms.html
> Well, here's my patch. This was a fun little exercise. The modifier  
> is called plural, and the syntax is explained at length in the  
> comments.

Nifty, this is very cute.

To answer the previous question better, a polish translation could use  
something like this:

#define PLURAL(N, A, B, C, D) \
    "%plural{1:" A "|%100=(10,20):" B "|%10=(2,4):" C "|:" D "}" #N


DIAG(...
       "someone needs %0 " PLURAL(0, "cheese", "cheeze", "cheices",  
"choiss") " to pass to this function")


Some specific requests:

+/// range      := number
+///             | '(' number ',' number ')'   -> ranges are inclusive  
both ends

for inclusive ranges, please use [] instead of ().  () are typically  
used for exclusive ranges.


+  while (Argument != ArgumentEnd) {
..
+  }
+  assert(false && "Plural expression didn't match.");


Please use:
   while (1) {
     assert(Argument != ArgumentEnd && ...
     ...
   }


+    while (*ExprEnd != ':') {
+      assert(ExprEnd != ArgumentEnd && "Plural missing expression  
end");
+      ++ExprEnd;
+    }

You don't like std::find? :)

If you move HandlePluralModifier lower in the file, you can avoid the  
forward declarations.

+    if(C == '%') {

Please put spaces after if keywords for consistency with the rest of  
the code.


+    } else if(C == '(' || (C >= '0' && C <= '9')) {
+      // Range expression
+      if (TestPluralRange(ValNo, Start, End))
+        return true;
+    } else {
+      assert(false && "Bad plural expression syntax: unexpected  
character");
+    }

Please use:

    } else {
      assert( what we know is true here)

    }


+    // Scan for next or-expr part.
+    while (Start != End && *Start != ',')
+      ++Start;

std::find please.


>> Third, please make sure you have a testcase for this so that we  
>> know if something breaks.
> I've got a small test program which uses a copy of the code, but how  
> do I add a proper test case for this feature? My small program isn't  
> run as part of the regressions, and is bound to go out of sync.

Is there a case in english that can use this?  mice vs mouse?  Worst  
case, you could convert a %s over to use this.

-Chris