Patchwork More ObjC++ work on @encode in C++ templates

login
register
mail settings
Submitter Nicola Pero
Date Oct. 7, 2010, 6:30 p.m.
Message ID <1286476223.05799754@192.168.2.228>
Download mbox | patch
Permalink /patch/67089/
State New
Headers show

Comments

Nicola Pero - Oct. 7, 2010, 6:30 p.m.
This patch completes the work to get @encode working in C++ templates.
I had to finish it myself as there was nothing more to merge from the
apple/trunk branch on the matter. :-)

Ok to apply ?

Thanks
Mike Stump - Oct. 7, 2010, 6:52 p.m.
On Oct 7, 2010, at 11:30 AM, Nicola Pero wrote:
> This patch completes the work to get @encode working in C++ templates.
> I had to finish it myself as there was nothing more to merge from the
> apple/trunk branch on the matter. :-)
> 
> Ok to apply ?

Ok.

In general, please try and avoid collisions on the testcase names with those names from llvm-gcc-4.2...  If they have an encode-10.mm testcase, but not an encode-15, go for encode-15.  The reason is that once we are done with the basics, one can then see which testcases are in that tree to see what features and functionalities we are still missing.  We may well have to reimplement the feature, but at least we can have a quick pointer to what we are missing.
Nicola Pero - Oct. 7, 2010, 10:33 p.m.
> In general, please try and avoid collisions on the testcase names with those names 
> from llvm-gcc-4.2...  If they have an encode-10.mm testcase, but not an encode-15, 
> go for encode-15.  The reason is that once we are done with the basics, one can 
> then see which testcases are in that tree to see what features and functionalities 
> we are still missing.  We may well have to reimplement the feature, but at least 
> we can have a quick pointer to what we are missing.

Sure.

Alternatively, maybe we should add new testcases (that we're not backporting from
the apple/trunk branch) to a new directory ?  Then we would never have an issue with 
conflicts. :-)

I think IainS was reorganizing the ObjC/ObjC++ testcases, maybe he has a recommendation
of where to add new testcases ?

Thanks
IainS - Oct. 7, 2010, 11:23 p.m.
On 7 Oct 2010, at 23:33, Nicola Pero wrote:

>
>> In general, please try and avoid collisions on the testcase names  
>> with those names
>> from llvm-gcc-4.2...  If they have an encode-10.mm testcase, but  
>> not an encode-15,
>> go for encode-15.  The reason is that once we are done with the  
>> basics, one can
>> then see which testcases are in that tree to see what features and  
>> functionalities
>> we are still missing.  We may well have to reimplement the feature,  
>> but at least
>> we can have a quick pointer to what we are missing.
>
> Sure.
>
> Alternatively, maybe we should add new testcases (that we're not  
> backporting from
> the apple/trunk branch) to a new directory ?  Then we would never  
> have an issue with
> conflicts. :-)
>
> I think IainS was reorganizing the ObjC/ObjC++ testcases, maybe he  
> has a recommendation
> of where to add new testcases ?

I can see the merit in making it easy to see what we've still got left  
to do ...
... and it makes good sense to use distinct names for new tests [maybe  
even put fsf at the front?]

... but it would be really really nice to avoid one huge dir that does  
all tests (where the differentiation in topic is solely by filename)...

Recently, I have been trying to add new stuff (where there are more  
than a few tests - or expected to be more than a few tests) to a new  
topic subdir from obj*.dg/
[e.g tls/, attributes/ .. and soon property/ ...  (I would prob. have  
added foreach/ ;-)) ]
Similarly, we can do that for torture tests under obj*.dg/torture/

we are a small group - so it shouldn't be too hard to debate what  
merits a separate category.

one then needs to identify all the tests in an area from LLVM and  
compare with the same area in our suite
- that is a little work - but relatively small work c.f. doing the  
coding.

IMO the long-term benefit of organizing the tests in our suite  
outweighs that little work in identifying the set of tests for a given  
topic in the LLVM structure.
(some form of tracking of additions/changes would be beneficial too -  
but I don't know that we've got enough resources to implement that).

---

I'd also like:
a/ to move the old objc/{compile,execute} to dg ... it is more  
flexible and makes ObjC++ consistent w/ObjC
b/ to  figure out a method for doing ObjC* common tests as is done  
with c/c++  - there are a lot of duplicates that could be merged.
c/ make sure that we are torture-testing enough stuff (e.g. ObJC++ had  
no optimized tests until recently, which was hiding a global fail).
d/ make one sub-structure for each runtime [and make sure we've  
covered the code-gen] since we're heading for 4 runtimes to test ;-)

just my £0.02...
Iain


P.S. even if it were feasible license-wise (which currently looks  
unlikely) - one cannot simply symlink to the LLVM dir and run the tests
-- there is too much divergence in the testsuite infrastructure.

PPS I guess we can always use find to flatten our dir structure and  
LLVM's and compare file lists that way.
Nicola Pero - Oct. 8, 2010, 7:43 a.m.
> ... but it would be really really nice to avoid one huge dir that does  
> all tests (where the differentiation in topic is solely by filename)...
> 
> Recently, I have been trying to add new stuff (where there are more  
> than a few tests - or expected to be more than a few tests) to a new  
> topic subdir from obj*.dg/
> [e.g tls/, attributes/ .. and soon property/ ...  (I would prob. have  
> added foreach/ ;-)) ]
> Similarly, we can do that for torture tests under obj*.dg/torture/

Sure, I love the idea of using subdirectories.  We could put all the "old"
tests into a "misc" or "old" subdirectory.  So, we reorganize tests as in:

 objc.dg/misc - all the "old" tests that are in common with llvm-gcc-4.2 so Mike
(and everyone else) can easily do the test comparison he wants to do.  Tests we
backport from apple/trunk branch would end in here.

 objc.dg/fast_enumeration - all the new fast enumeration tests

 objc.dg/encode - all the new encode tests

 etc. all new tests would go into an appropriate subdirectory

Over time we'd end up with something similar to testsuite/g++.dg/ where they 
have testcases neatly organized in subdirectories.



> I'd also like:
> a/ to move the old objc/{compile,execute} to dg ... it is more  
> flexible and makes ObjC++ consistent w/ObjC
> b/ to  figure out a method for doing ObjC* common tests as is done  
> with c/c++  - there are a lot of duplicates that could be merged.
> c/ make sure that we are torture-testing enough stuff (e.g. ObJC++ had  
> no optimized tests until recently, which was hiding a global fail).
> d/ make one sub-structure for each runtime [and make sure we've  
> covered the code-gen] since we're heading for 4 runtimes to test ;-)

Sounds good.


> P.S. even if it were feasible license-wise (which currently looks  
> unlikely) - one cannot simply symlink to the LLVM dir and run the tests
> -- there is too much divergence in the testsuite infrastructure.

I think Mike was referring to llvm-gcc-4.2, which looks like having the 
same testsuite as we do - just with more ObjC/ObjC++ tests.  And I'd love 
to take our ObjC/ObjC++ compiler and run it on the ObjC/ObjC++ testsuite 
of another compiler to look for bugs or features to implement. ;-)
IainS - Oct. 8, 2010, 8:55 a.m.
On 8 Oct 2010, at 08:43, Nicola Pero wrote:

>
>> ... but it would be really really nice to avoid one huge dir that  
>> does
>> all tests (where the differentiation in topic is solely by  
>> filename)...
>>
>> Recently, I have been trying to add new stuff (where there are more
>> than a few tests - or expected to be more than a few tests) to a new
>> topic subdir from obj*.dg/
>> [e.g tls/, attributes/ .. and soon property/ ...  (I would prob. have
>> added foreach/ ;-)) ]
>> Similarly, we can do that for torture tests under obj*.dg/torture/
>
> Sure, I love the idea of using subdirectories.  We could put all the  
> "old"
> tests into a "misc" or "old" subdirectory.  So, we reorganize tests  
> as in

we would, in any case, end up needing a misc dir - since it is  
inefficient to have only one or two tests in a subdir (all the  
overhead of a new .exp).

> objc.dg/misc - all the "old" tests that are in common with llvm- 
> gcc-4.2 so Mike
> (and everyone else) can easily do the test comparison he wants to  
> do.  Tests we
> backport from apple/trunk branch would end in here.

I don't favor duplicating tests - and IMO, in the end, for both old  
and new developers it will be easier to look for all tests for  
'encode' in a sub-dir named 'encode' ... ;-)
[even if the dev. knew that the tests used to live in /objc.dg ]

> objc.dg/fast_enumeration - all the new fast enumeration tests
>
> objc.dg/encode - all the new encode tests

great :-)

> etc. all new tests would go into an appropriate subdirectory

hmmmm  ....
I'd favor keeping all 'topic-x' tests together --  that's what helps  
the dev. to find what's been tested and what hasn't ...

.... and to allow comparison with external sources - adopt a  
convention that differentiates new from old & imported.
(maybe agree to prefix new tests with "fsf-" ?   --   so that we don't  
need to worry about name clashes with future LLVM tests)

I doubt we can import LLVM tests, for the same reason we can't import  
the code -- so the only tests to be 'imported' are from the FSF apple/ 
trunk branch.

We can keep the same names for old, and apple-local imports -  
therefore a simple 'find' stripping the path prefixes should be able  
to flatten both our tree and LLVM making comparison easy.

>> P.S. even if it were feasible license-wise (which currently looks
>> unlikely) - one cannot simply symlink to the LLVM dir and run the  
>> tests
>> -- there is too much divergence in the testsuite infrastructure.
>
> I think Mike was referring to llvm-gcc-4.2, which looks like having  
> the
> same testsuite as we do - just with more ObjC/ObjC++ tests.  And I'd  
> love
> to take our ObjC/ObjC++ compiler and run it on the ObjC/ObjC++  
> testsuite
> of another compiler to look for bugs or features to implement. ;-)

it's not just the layout --
  -- the functionality of lib/*.exp has diverged and some of the  
mechanisms expected from 4.2 era tests don't work with 4.6 era  
realities.

(IIRC the Makefiles don't do quite the same things -- e.g. there is no  
ability to do check-gcc-objc from the top level).

cheers,
Iain
Nicola Pero - Oct. 8, 2010, 12:30 p.m.
>> Sure, I love the idea of using subdirectories.  We could put all the  
>> "old"
>> tests into a "misc" or "old" subdirectory.  So, we reorganize tests  
>> as in
>
> we would, in any case, end up needing a misc dir - since it is  
> inefficient to have only one or two tests in a subdir (all the  
> overhead of a new .exp).

I briefly looked at g++.dg/ and they have a single dg.exp which runs
all the tests in all the subdirectories (with the exceptions of the directories
that run special tests).

So there should be no particular overhead to using subdirectories; even multiple
levels. :-)

So, maybe we could use

 objc.dg/encode/base/... <encode tests shared with the apple/trunk branch>
 objc.dg/encode/...      <new encode tests>

 objc.dg/fast-enumeration/base/... <fast enumeration tests shared with the apple/trunk branch>
 objc.dg/fast-enumeration/...      <new fast enumeration tests>

 objc.dg/{topic}/base/...          <test on {topic} shared with the apple/trunk branch>
 objc.dg/{topic}/...               <new tests on {topic} that we add>

That should take care of your concern that all tests for fast enumeration (for example)
should be in the same place.

It would still be easy to compare our list of tests with the one from llvm-gcc-4.2 (as Mike
wants to be able to), as you would simply do

ls objc.dg/*/base/

to get the list of our tests shared with the apple/trunk branch (you don't even need find), which
you can immediately compare with the list in llvm-gcc-4.2 (and, there won' be any confusing clashes
due to names).

Anyway, let's continue the discussion when you are back online in a few days. :-)

Thanks
Mike Stump - Oct. 11, 2010, 6:51 p.m.
On Oct 8, 2010, at 12:43 AM, Nicola Pero wrote:
> Sure, I love the idea of using subdirectories.  We could put all the "old"
> tests into a "misc" or "old" subdirectory.

old is never a good name.  Neither is new.

The usual convention is to leave the testsuite as is and not to `reorganize' it.  There is little benefit from the reorganization and the benefit of being able to compare and track failures and regressions with stable testcase names is at time useful.  Also, using the stable names to be able to compare with llvm-gcc-4.2 is useful.  I'd like to see that preserved.  Now, if we didn't have any functionality remaining to be merged, and there were 0 testcase failures on all platforms, the benefit of the stable names lessons, but, I don't think we are there yet.
Nicola Pero - Oct. 15, 2010, 12:46 p.m.
> The usual convention is to leave the testsuite as is and not to `reorganize' it.

I agree that we have more pressing things to do with Objective-C than reorganizing
the testsuite :-)

I still think it would be worth doing it (there are some very practical benefits), 
but I understand you may feel it's a waste of time at this stage and, considering
there are only few weeks left before the end of stage 1, I agree and won't insist
or spend more time discussing it. ;-)

Let's ignore it and maybe pick it up at the next release.

Thanks

Patch

Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog     (revision 165128)
+++ gcc/testsuite/ChangeLog     (working copy)
@@ -1,3 +1,7 @@ 
+2010-10-07  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * obj-c++.dg/encode-10.mm: New testcase.
+
 2010-10-07  Janus Weil  <janus@gcc.gnu.org>
 
        PR fortran/45933
Index: gcc/testsuite/obj-c++.dg/encode-10.mm
===================================================================
--- gcc/testsuite/obj-c++.dg/encode-10.mm       (revision 0)
+++ gcc/testsuite/obj-c++.dg/encode-10.mm       (revision 0)
@@ -0,0 +1,46 @@ 
+/* Test for @encode in templates.  */
+/* { dg-options "-lobjc" } */
+/* { dg-do run } */
+#include <string.h>           
+#include <stdlib.h>
+
+template<typename T>
+const char *my_encode(int variant)
+{
+  const char *result;
+
+  switch (variant)
+    {
+    case 0:
+      result = @encode(T);
+      break;
+    case 1:
+      result = @encode(T*);
+      break;
+    case 2:
+      result = @encode(const T*);
+      break;
+    default:
+      result = @encode(int);
+      break;
+    }
+
+  return result;
+}
+
+int main()
+{
+  if (strcmp (@encode(char), my_encode<char>(0)))
+    abort ();
+
+  if (strcmp (@encode(char *), my_encode<char>(1)))
+    abort ();
+
+  if (strcmp (@encode(const char *), my_encode<char>(2)))
+    abort ();
+
+  if (strcmp (@encode(int), my_encode<char>(3)))
+    abort ();
+
+  return 0;
+}
Index: gcc/cp/error.c
===================================================================
--- gcc/cp/error.c      (revision 165128)
+++ gcc/cp/error.c      (working copy)
@@ -2141,6 +2141,14 @@  dump_expr (tree t, int flags)
       pp_cxx_right_paren (cxx_pp);
       break;
 
+    case AT_ENCODE_EXPR:
+      pp_cxx_ws_string (cxx_pp, "@encode");
+      pp_cxx_whitespace (cxx_pp);
+      pp_cxx_left_paren (cxx_pp);
+      dump_type (TREE_OPERAND (t, 0), flags);
+      pp_cxx_right_paren (cxx_pp);
+      break;
+
     case NOEXCEPT_EXPR:
       pp_cxx_ws_string (cxx_pp, "noexcept");
       pp_cxx_whitespace (cxx_pp);
Index: gcc/cp/ChangeLog
===================================================================
--- gcc/cp/ChangeLog    (revision 165128)
+++ gcc/cp/ChangeLog    (working copy)
@@ -1,3 +1,15 @@ 
+2010-10-07  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * cp-tree.def: Changed type of AT_ENCODE_EXPR from tcc_unary to
+       tcc_expression.
+       * cxx-pretty-print.c (pp_cxx_unary_expression): Added case for
+       AT_ENCODE_EXPR.
+       * error.c (dump_expr): Added case for AT_ENCODE_EXPR.
+       * pt.c (tsubst_copy): Added case for AT_ENCODE_EXPR.
+       (value_dependent_expression_p): Added case for AT_ENCODE_EXPR.
+       (type_dependent_expression_p): Added case for AT_ENCODE_EXPR.
+       * parser.c (cp_parser_objc_encode_expression): Updated comment.
+       
 2010-10-07  Nicola Pero  <nicola@nicola.brainstorm.co.uk>
 
        Merge from apple/trunk branch on FSF servers.
Index: gcc/cp/cxx-pretty-print.c
===================================================================
--- gcc/cp/cxx-pretty-print.c   (revision 165128)
+++ gcc/cp/cxx-pretty-print.c   (working copy)
@@ -791,6 +791,14 @@  pp_cxx_unary_expression (cxx_pretty_printer *pp, t
        pp_unary_expression (pp, TREE_OPERAND (t, 0));
       break;
 
+    case AT_ENCODE_EXPR:
+      pp_cxx_ws_string (pp, "@encode");
+      pp_cxx_whitespace (pp);
+      pp_cxx_left_paren (pp);
+      pp_cxx_type_id (pp, TREE_OPERAND (t, 0));
+      pp_cxx_right_paren (pp);
+      break;      
+
     case NOEXCEPT_EXPR:
       pp_cxx_ws_string (pp, "noexcept");
       pp_cxx_whitespace (pp);
Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c (revision 165128)
+++ gcc/cp/pt.c (working copy)
@@ -11131,6 +11131,7 @@  tsubst_copy (tree t, tree args, tsubst_flags_t com
     case ADDR_EXPR:
     case UNARY_PLUS_EXPR:      /* Unary + */
     case ALIGNOF_EXPR:
+    case AT_ENCODE_EXPR:
     case ARROW_EXPR:
     case THROW_EXPR:
     case TYPEID_EXPR:
@@ -17689,6 +17690,12 @@  value_dependent_expression_p (tree expression)
        return dependent_type_p (expression);
       return type_dependent_expression_p (expression);
 
+    case AT_ENCODE_EXPR:
+      /* An 'encode' expression is value-dependent if the operand is
+        type-dependent.  */
+      expression = TREE_OPERAND (expression, 0);
+      return dependent_type_p (expression);
+
     case NOEXCEPT_EXPR:
       expression = TREE_OPERAND (expression, 0);
       /* FIXME why check value-dependency?  */
@@ -17806,6 +17813,7 @@  type_dependent_expression_p (tree expression)
   if (TREE_CODE (expression) == PSEUDO_DTOR_EXPR
       || TREE_CODE (expression) == SIZEOF_EXPR
       || TREE_CODE (expression) == ALIGNOF_EXPR
+      || TREE_CODE (expression) == AT_ENCODE_EXPR
       || TREE_CODE (expression) == NOEXCEPT_EXPR
       || TREE_CODE (expression) == TRAIT_EXPR
       || TREE_CODE (expression) == TYPEID_EXPR
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c     (revision 165128)
+++ gcc/cp/parser.c     (working copy)
@@ -21128,6 +21128,12 @@  cp_parser_objc_encode_expression (cp_parser* parse
       return error_mark_node;
     }
 
+  /* This happens if we find @encode(T) (where T is a template
+     typename or something dependent on a template typename) when
+     parsing a template.  In that case, we can't compile it
+     immediately, but we rather create an AT_ENCODE_EXPR which will
+     need to be instantiated when the template is used.
+  */
   if (dependent_type_p (type))
     {
       tree value = build_min (AT_ENCODE_EXPR, size_type_node, type);
Index: gcc/cp/cp-tree.def
===================================================================
--- gcc/cp/cp-tree.def  (revision 165128)
+++ gcc/cp/cp-tree.def  (working copy)
@@ -337,7 +337,7 @@  DEFTREECODE (ALIGNOF_EXPR, "alignof_expr", tcc_exp
 
 /* Represents an Objective-C++ '@encode' expression during template
    expansion.  */
-DEFTREECODE (AT_ENCODE_EXPR, "at_encode_expr", tcc_unary, 1)
+DEFTREECODE (AT_ENCODE_EXPR, "at_encode_expr", tcc_expression, 1)
 
 /* A STMT_EXPR represents a statement-expression during template
    expansion.  This is the GCC extension { ( ... ) }.  The