clarification: MYTHREAD and THREADS are expressions (cannot assign to or take address of them)

[GoogleCodeExporter]

This proposal addresses item #4 on the UPC Language Specification Issues list.

The question has 2 parts:
1) *CAN* MYTHREAD and THREADS be specified as "const"?
2) *SHOULD* MYTHREAD and THREADS be specified as "const"?

In my opinion the answer to part 2 is YES.
I say that because in some (many?) runtime implementations it is possible to  
accidentally modify either of these.  It is therefore a good thing to protect 
against errors like "=" where "==" was intended.

Part 1 of the question takes a little bit more thought, but I could not find 
any VALID code construct that would become illegal by virtue of making either 
of these identifiers const.  I *can*, however, construct places in which they 
might be passed by reference and WARNINGS would result from the implicit 
conversion of (const int *) to (int *).

I propose to make MYTHREADS and THREADS "const" in the UPC 1.3 spec.
If the change is agreeable, I will follow up with specific wording and its 
placement at a later date. 

Original issue reported on code.google.com by phhargr...@lbl.gov on 22 May 2012 at 3:03

[GoogleCodeExporter]

> Part 1 of the question takes a little bit more thought, but I could not find 
any
> VALID code construct that would become illegal by virtue of making either of 
these
> identifiers const.  I *can*, however, construct places in which they might be 
passed
> by reference and WARNINGS would result from the implicit conversion of
> (const int *) to (int *).

Perhaps one argument against defining MYTHREAD (for example) as:
  const int MYTHREAD;
Is that it might be defined as a macro, which invokes perhaps a function (eg, 
get_my_rank()).

Currently, MYTHREAD is defined as follows.

"MYTHREAD is an expression with a value of type int; it specifies the unique
thread index. The range of possible values is 0..THREADS-1"

Personally, I'd like to see both THREADS and MYTHREAD defined as "const" for 
the reasons cited in the issue description, however, it is worth noting that 
the current definition allows both MYTHREAD and THREADS as a macro (at least in 
a dynamic THREADS environment).

This current characteristic of THREADS is also potentially problematic.

"Under the static THREADS translation environment, THREADS is an integer 
constant suitable for use in #if preprocessing directives."

THREADS is a constant in the static THREADS compilation environment, but it is 
also a  constant value that can be tested in a #if directive.

Original comment by gary.funck on 22 May 2012 at 4:00

[GoogleCodeExporter]

Gary writes:
> Perhaps one argument against defining MYTHREAD (for example) as:
>  const int MYTHREAD;
> Is that it might be defined as a macro, which invokes perhaps a function
> (eg, get_my_rank()).

Indeed this is how Berkeley UPC implements MYTHREAD, and THREADS in a dynamic 
environment is also a function call.  However, since we are source-to-source 
that is the "back end" of the implementation.  Nothing in my understanding of 
the Berkeley compiler would prevent us from declaring "const int MYTHREAD" and 
"const int THREADS" in upc.h and still keeping the current implementation.

Since I cannot claim to speak for other implementers, I'll pose this as a 
question: 

+ Do you see significant technical difficulties in implementing MYTHREAD as 
having type (const int)?
+ Do you see significant technical difficulties in implementing THREADS as 
having type (const int) in the dynamic threads environment?  (It would, I now 
see, need to remain a preprocessor macro under static threads).

Original comment by phhargr...@lbl.gov on 22 May 2012 at 4:36

[GoogleCodeExporter]

Paul asked:

+ Do you see significant technical difficulties in implementing MYTHREAD as 
having type (const int)?
+ Do you see significant technical difficulties in implementing THREADS as 
having type (const int) in the dynamic threads environment?

No significant difficulties for HP UPC.

Original comment by brian.wibecan on 22 May 2012 at 8:59

[GoogleCodeExporter]

I think some subtle points of the current definition for MYTHREAD and THREADS 
are being missed by this issue.

Currently, MYTHREAD and THREADS are both defined as "EXPRESSIONS with a value 
of type int". It's very important to note they are NOT defined as objects. 
Consequently by ISO C 6.3.2.1 they are NOT L-values. Consequently, it is 
*already* illegal to assign a value to them (by ISO C 6.5.16 "An assignment 
operator shall have a modifiable lvalue as its left operand."), in the same way 
that it is illegal to write: (1+2) = 7;

They was specified in this manner for several reasons:
* It prohibits assignment
* It prohibits address-of
* It allows their implementation as macroed function calls

Const is meaningless and unnecessary on the base type of an expression, so this 
issue is not a necessary change. However as this is admittedly subtle logic we 
should perhaps provide clarification in the form of a footnote indicating that 
they are not lvalues.

Original comment by danbonachea on 31 May 2012 at 5:36

[GoogleCodeExporter]

As owner of this issue, and based on Dan's observation that "expression" is not 
equivalent to an object, I propose that in UPC 1.3 we should:

Add clarification that MYTHREAD and THREADS are *not* L-values.
A footnote seems appropriate for this change:
   [n] The definition of MYTHREAD and THREADS as expressions means
       one cannot assign to them or take their address.

For any implementation using an object, I would suggest:
  extern int _mythread;
  #define MYTHREAD (_mythread+0)
  #ifdef __UPC_DYNAMIC_THREADS__
     extern int _threads;
  #  define THREADS (_threads+0)
  #endif
as mechanism to ensure one CANNOT assign to ,or take the address of, these 
identifiers.

Original comment by phhargr...@lbl.gov on 31 May 2012 at 6:18

[GoogleCodeExporter]

Is there an implementation "trick" that can be used in the example in comment 
#5 above, is there any other expression that has a higher chance of not require 
evaluation by generated code (for example in the case of a compiler that 
doesn't detect addition of zero as a no-op?

For example:
  #define THREADS +_threads

(GCC may have a builtin for this, would need to check.)

Original comment by gary.funck on 31 May 2012 at 6:34

[GoogleCodeExporter]

Gary wrote:

> For example:
>  #define THREADS +_threads

That would change "7 THREADS" from being a syntax error to being valid code!
At a minimum one needs parens:
   #define THREADS (+_threads)

Other than the parens, I don't see a problem with Gary's suggestion.

Another option:
  #define THREADS ((int)_threads)
where "int" could be replaced by upc_thread_t as desired/required.
Though I seem to recall that gcc USED TO allow "((int)a) = 0".

IMHO: a compiler that can optimize away addition of zero seems questionable.

Original comment by phhargr...@lbl.gov on 31 May 2012 at 6:52

[GoogleCodeExporter]

Um, in my previous comment the text
  IMHO: a compiler that can optimize away addition of zero seems questionable.
should have said "cannot" rather than "can".

Original comment by phhargr...@lbl.gov on 31 May 2012 at 6:55

[GoogleCodeExporter]

Original comment by phhargr...@lbl.gov on 1 Jun 2012 at 3:19

• Added labels: Spec-1.3

[GoogleCodeExporter]

Original comment by phhargr...@lbl.gov on 1 Jun 2012 at 6:09

• Added labels: Milestone-Spec-1.3
• Removed labels: Spec-1.3

[GoogleCodeExporter]

Changing the summary to reflect the current nature of the proposal

Original comment by phhargr...@lbl.gov on 14 Jun 2012 at 11:07

• Changed title: clarification: MYTHREAD and THREADS are expressions (cannot assign to or take address of them)

[GoogleCodeExporter]

I'm in favor of Paul's proposed footnote clarification in comment 5 as a 
resolution to this issue.

The separate discussion about implementation tricks isn't relevant to spec 
language.

Original comment by danbonachea on 15 Jun 2012 at 7:19

[GoogleCodeExporter]

Original comment by gary.funck on 3 Jul 2012 at 3:10

• Added labels: Type-Clarification
• Removed labels: Type-Defect

[GoogleCodeExporter]

Here is the proposed change:

--- upc-language.tex    (revision 80)
+++ upc-language.tex    (working copy)
@@ -79,7 +79,10 @@
 \subsubsection{{\tt MYTHREAD}}
 \index{MYTHREAD}
 \npf{\tt MYTHREAD} is an expression with a value of type int; it specifies the
-     unique thread index. The range of possible values is {\tt
+     unique thread index.%
+     \footnote{The definition of MYTHREAD and THREADS as expressions means
+               one cannot assign to them or take their address.}
+     The range of possible values is {\tt
      0..THREADS-1}\footnote{e.g., the program {\tt main()\{
      printf("\%d ",MYTHREAD); \} }, prints the numbers 0 through
      THREADS-1, in some order.}.

Original comment by danbonachea on 13 Aug 2012 at 3:24

[GoogleCodeExporter]

I wrote the proposed change, but am now wondering is if this is formal enough.

Does "cannot be used as an lvalue" sound better?
That automatically prohibits both assignment and address-of.
However, it might be too subtle.

Original comment by phhargr...@lbl.gov on 13 Aug 2012 at 3:29

[GoogleCodeExporter]

This is just a clarification, so I think it's best to directly state the 
clarification we're trying to make. Assignment and address-taking are the two 
actions we are trying to clarify are forbidden.

Would this be clearer?:

\footnote{The definition of MYTHREAD and THREADS as expressions, not objects or 

          l-values, means one cannot assign to them or take their address.}

Original comment by danbonachea on 13 Aug 2012 at 3:40

[GoogleCodeExporter]

Yes, the more verbose text of comment #16 seems superior to me.

Original comment by phhargr...@lbl.gov on 13 Aug 2012 at 3:52

[GoogleCodeExporter]

Updated diff:

--- upc-language.tex    (revision 80)
+++ upc-language.tex    (working copy)
@@ -79,7 +79,10 @@
 \subsubsection{{\tt MYTHREAD}}
 \index{MYTHREAD}
 \npf{\tt MYTHREAD} is an expression with a value of type int; it specifies the
-     unique thread index. The range of possible values is {\tt
+     unique thread index.%
+     \footnote{The definition of MYTHREAD and THREADS as expressions, not 
objects or   
+               l-values, means one cannot assign to them or take their 
address.}
+     The range of possible values is {\tt
      0..THREADS-1}\footnote{e.g., the program {\tt main()\{
      printf("\%d ",MYTHREAD); \} }, prints the numbers 0 through
      THREADS-1, in some order.}.

Original comment by danbonachea on 13 Aug 2012 at 4:36

[GoogleCodeExporter]

Mass-edit to mark all issues with an officially-announced resolution diff as 
"PendingApproval"

Original comment by danbonachea on 17 Aug 2012 at 3:25

• Changed state: PendingApproval

[GoogleCodeExporter]

Set Consensus:High on Pending Approval issues.

Original comment by gary.funck on 19 Aug 2012 at 11:31

• Added labels: Consensus-High

[GoogleCodeExporter]

Comment #18 committed as SVN r108

Original comment by danbonachea on 14 Sep 2012 at 12:15

• Changed state: Fixed

[GoogleCodeExporter]

For the record:
Public comment period for this change was 8/14/2012 - 9/14/2012
No substantial objections were raised or recorded during that period.
At the 9/7/2012 telecon, it was announced this change was imminent, feedback 
was explicitly solicited, and none was received.
The change was integrated into a working draft distributed on 9/13/2012 for 
consideration and draft ratification at the 9/14/2012 telecon.

Original comment by danbonachea on 14 Sep 2012 at 7:26

[GoogleCodeExporter]

Changes distributed as Draft 1.1 were ratified during the 9/14/2012 
teleconference, for inclusion in the next public draft.

Original comment by danbonachea on 14 Sep 2012 at 9:46

• Changed state: Ratified