Skip to content

Commit

Permalink
Merge pull request #19983 from benharsh/deprecate-readWriteThis
Browse files Browse the repository at this point in the history
Deprecate readWriteThis methods

This PR updates various modules and tests to use ``readThis`` and ``writeThis`` instead of ``readWriteThis`` methods. Existing methods that weren't no-doc'd are left in place and given their own deprecation warning.

This PR also adds a new future for reading a ``map``: test/library/standard/Map/testMapIO.future

[reviewed-by @lydia-duncan]
  • Loading branch information
benharsh authored Jun 15, 2022
2 parents 2e15eb1 + 1964692 commit f6eb173
Show file tree
Hide file tree
Showing 30 changed files with 430 additions and 105 deletions.
5 changes: 4 additions & 1 deletion compiler/passes/buildDefaultFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1825,7 +1825,10 @@ static void buildDefaultReadWriteFunctions(AggregateType* ct) {
return;

// If we have a readWriteThis, we'll call it from readThis/writeThis.
if (functionExists("readWriteThis", dtMethodToken, ct, dtAny)) {
if (FnSymbol* fn = functionExists("readWriteThis", dtMethodToken, ct, dtAny)) {
if (fn->hasFlag(FLAG_DEPRECATED) == false) {
USR_WARN(fn, "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead.");
}
hasReadWriteThis = true;
}

Expand Down
8 changes: 6 additions & 2 deletions modules/internal/ChapelArray.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,12 @@ module ChapelArray {

proc idxToLocale(ind) return _value.dsiIndexToLocale(ind);

proc readWriteThis(f) throws {
f <~> _value;
proc readThis(f) throws {
f.read(_value);
}

proc writeThis(f) throws {
f.write(_value);
}

proc displayRepresentation() { _value.dsiDisplayRepresentation(); }
Expand Down
12 changes: 11 additions & 1 deletion modules/internal/OwnedObject.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -441,9 +441,19 @@ module OwnedObject {
__primitive("call destructor", __primitive("deref", x));
}

pragma "no doc"
proc _owned.readThis(f) throws {
_readWriteHelper(f);
}

pragma "no doc"
proc _owned.writeThis(f) throws {
_readWriteHelper(f);
}

// Don't print out 'chpl_p' when printing an _owned, just print class pointer
pragma "no doc"
proc _owned.readWriteThis(f) throws {
proc _owned._readWriteHelper(f) throws {
if isNonNilableClass(this.chpl_t) {
var tmp = this.chpl_p! : borrowed class;
f <~> tmp;
Expand Down
12 changes: 11 additions & 1 deletion modules/internal/SharedObject.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -496,9 +496,19 @@ module SharedObject {
__primitive("call destructor", __primitive("deref", x));
}

pragma "no doc"
proc _shared.readThis(f) throws {
_readWriteHelper(f);
}

pragma "no doc"
proc _shared.writeThis(f) throws {
_readWriteHelper(f);
}

// Don't print out 'chpl_p' when printing an Shared, just print class pointer
pragma "no doc"
proc _shared.readWriteThis(f) throws {
proc _shared._readWriteHelper(f) throws {
if isNonNilableClass(this.chpl_t) {
var tmp = this.chpl_p! : borrowed class;
f <~> tmp;
Expand Down
27 changes: 25 additions & 2 deletions modules/packages/AtomicObjects.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,22 @@ prototype module AtomicObjects {
return __ABA_cnt;
}

proc readWriteThis(f) throws {
pragma "no doc"
proc readThis(f) throws {
compilerWarning("Reading an ABA is not supported");
}

/* Writes an ABA */
proc writeThis(f) throws {
f <~> "(ABA){cnt=" <~> this.__ABA_cnt <~> ", obj=" <~> this.getObject() <~> "}";
}

deprecated "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead."
proc readWriteThis(f) throws {
if f.writing then writeThis(f);
else readThis(f);
}

forwarding this.getObject()!;
}
operator ABA.=(ref lhs: ABA, const ref rhs: lhs.type) {
Expand Down Expand Up @@ -618,8 +630,19 @@ prototype module AtomicObjects {
return ret;
}

proc readWriteThis(f) throws {
pragma "no doc"
proc readThis(f) throws {
compilerWarning("Reading an AtomicObject is not supported");
}

proc writeThis(f) throws {
f <~> atomicVariable.read();
}

deprecated "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead."
proc readWriteThis(f) throws {
if f.writing then writeThis(f);
else readThis(f);
}
}
}
22 changes: 21 additions & 1 deletion modules/packages/ConcurrentMap.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,11 @@ module ConcurrentMap {
return A;
}

pragma "no doc"
proc readThis(f) throws {
compilerWarning("Reading a ConcurrentMap is not supported");
}

/*
Writes the contents of this map to a channel. The format looks like:
Expand All @@ -1051,7 +1056,7 @@ module ConcurrentMap {
:arg ch: A channel to write to.
*/
proc readWriteThis(ch: channel) throws {
proc writeThis(f) throws {
ch <~> "{";
var first = true;
for (key, val) in this {
Expand All @@ -1064,6 +1069,21 @@ module ConcurrentMap {
}
ch <~> "}";
}

/*
Writes the contents of this map to a channel. The format looks like:
.. code-block:: chapel
{k1: v1, k2: v2, .... , kn: vn}
:arg ch: A channel to write to.
*/
deprecated "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead."
proc readWriteThis(ch: channel) throws {
if ch.writing then writeThis(ch);
else readThis(ch);
}
}

/*
Expand Down
8 changes: 6 additions & 2 deletions modules/packages/DistributedBag.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,12 @@ module DistributedBag {
}

pragma "no doc"
/* Read/write the contents of DistBag from/to a channel */
proc readWriteThis(ch: channel) throws {
proc readThis(f) throws {
compilerError("Reading a DistBag is not supported");
}

pragma "no doc"
proc writeThis(ch) throws {
ch <~> "[";
var size = this.getSize();
for (i,iteration) in zip(this, 0..<size) {
Expand Down
7 changes: 6 additions & 1 deletion modules/packages/EpochManager.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,12 @@ module EpochManager {
return arr;
}

proc readWriteThis(f) throws {
pragma "no doc"
proc readThis(f) throws {
compilerError("Reading a Vector is not supported");
}

proc writeThis(f) throws {
f <~> "(Vector) {" <~> this.toArray() <~> "}";
}
}
Expand Down
7 changes: 6 additions & 1 deletion modules/packages/TOML.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -1363,7 +1363,12 @@ module TomlReader {
}
}

proc readWriteThis(f) throws {
pragma "no doc"
proc readThis(f) throws {
compilerError("Reading a Tokens type is not supported");
}

proc writeThis(f) throws {
// TODO: The `list` type currently doesn't support readWriteThis!
f <~> this.A.toArray();
}
Expand Down
14 changes: 13 additions & 1 deletion modules/standard/ChapelIO.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ of a Hello World program:
The readThis(), writeThis(), and readWriteThis() Methods
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.. warning:: 'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead.
When programming the input and output method for a custom data type, it is
often useful to define both the read and write routines at the same time. That
is possible to do in a Chapel program by defining a ``readWriteThis`` method,
Expand Down Expand Up @@ -637,9 +639,19 @@ module ChapelIO {
pragma "no doc"
proc nothing.writeThis(f) {}

pragma "no doc"
proc _tuple.readThis(f) throws {
_readWriteHelper(f);
}

pragma "no doc"
proc _tuple.writeThis(f) throws {
_readWriteHelper(f);
}

// Moved here to avoid circular dependencies in ChapelTuple.
pragma "no doc"
proc _tuple.readWriteThis(f) throws {
proc _tuple._readWriteHelper(f) throws {
var st = f.styleElement(QIO_STYLE_ELEMENT_TUPLE);
var start:ioLiteral;
var comma:ioLiteral;
Expand Down
126 changes: 74 additions & 52 deletions modules/standard/DateTime.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -495,27 +495,38 @@ module DateTime {

private use IO;

/* Read or write a date value from channel `f` */
proc date.readWriteThis(f) throws {
// This method exists to work around a bug in chpldoc where the
// 'private use' above this method somehow breaks documentation for the
// method that follows (formerly 'writeThis')
pragma "no doc"
proc date._chpldoc_workaround() { }

/* Writes this `date` in ISO 8601 format: YYYY-MM-DD */
proc date.writeThis(f) throws {
f.write(isoFormat());
}

/* Reads this `date` from ISO 8601 format: YYYY-MM-DD */
proc date.readThis(f) throws {
const dash = new ioLiteral("-");
const binary = f.binary(),
arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY),
isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary;

if f.writing {
try! {
f.write(isoFormat());
}
} else {
const binary = f.binary(),
arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY),
isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary;
if isjson then
f <~> new ioLiteral('"');

if isjson then
f <~> new ioLiteral('"');
f <~> chpl_year <~> dash <~> chpl_month <~> dash <~> chpl_day;

f <~> chpl_year <~> dash <~> chpl_month <~> dash <~> chpl_day;
if isjson then
f <~> new ioLiteral('"');
}

if isjson then
f <~> new ioLiteral('"');
}
/* Read or write a date value from channel `f` */
deprecated "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead."
proc date.readWriteThis(f) throws {
if f.writing then writeThis(f);
else readThis(f);
}


Expand Down Expand Up @@ -797,27 +808,33 @@ module DateTime {
return str;
}

/* Read or write a time value from channel `f` */
proc time.readWriteThis(f) throws {
/* Writes this `time` in ISO format: hh:mm:ss.sss */
proc time.writeThis(f) throws {
f.write(isoFormat());
}

/* Reads this `time` from ISO format: hh:mm:ss.sss */
proc time.readThis(f) throws {
const colon = new ioLiteral(":");
if f.writing {
try! {
f.write(isoFormat());
}
} else {
const binary = f.binary(),
arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY),
isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary;
const binary = f.binary(),
arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY),
isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary;

if isjson then
f <~> new ioLiteral('"');
if isjson then
f <~> new ioLiteral('"');

f <~> chpl_hour <~> colon <~> chpl_minute <~> colon <~> chpl_second
<~> new ioLiteral(".") <~> chpl_microsecond;
f <~> chpl_hour <~> colon <~> chpl_minute <~> colon <~> chpl_second
<~> new ioLiteral(".") <~> chpl_microsecond;

if isjson then
f <~> new ioLiteral('"');
}
if isjson then
f <~> new ioLiteral('"');
}

/* Read or write a time value from channel `f` */
deprecated "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead."
proc time.readWriteThis(f) throws {
if f.writing then writeThis(f);
else readThis(f);
}


Expand Down Expand Up @@ -1456,31 +1473,36 @@ module DateTime {
return this.strftime("%a %b %e %T %Y");
}

/* Read or write a datetime value from channel `f` */
proc datetime.readWriteThis(f) throws {
/* Writes this `datetime` in ISO format: YYYY-MM-DDThh:mm:ss.sss */
proc datetime.writeThis(f) throws {
f.write(isoFormat());
}

/* Reads this `datetime` from ISO format: YYYY-MM-DDThh:mm:ss.sss */
proc datetime.readThis(f) throws {
const dash = new ioLiteral("-"),
colon = new ioLiteral(":");
const binary = f.binary(),
arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY),
isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary;

if f.writing {
try! {
f.write(isoFormat());
}
} else {
const binary = f.binary(),
arrayStyle = f.styleElement(QIO_STYLE_ELEMENT_ARRAY),
isjson = arrayStyle == QIO_ARRAY_FORMAT_JSON && !binary;
if isjson then
f <~> new ioLiteral('"');

if isjson then
f <~> new ioLiteral('"');
f <~> chpl_date.chpl_year <~> dash <~> chpl_date.chpl_month <~> dash
<~> chpl_date.chpl_day <~> new ioLiteral("T") <~> chpl_time.chpl_hour
<~> colon <~> chpl_time.chpl_minute <~> colon <~> chpl_time.chpl_second
<~> new ioLiteral(".") <~> chpl_time.chpl_microsecond;

f <~> chpl_date.chpl_year <~> dash <~> chpl_date.chpl_month <~> dash
<~> chpl_date.chpl_day <~> new ioLiteral("T") <~> chpl_time.chpl_hour
<~> colon <~> chpl_time.chpl_minute <~> colon <~> chpl_time.chpl_second
<~> new ioLiteral(".") <~> chpl_time.chpl_microsecond;
if isjson then
f <~> new ioLiteral('"');
}

if isjson then
f <~> new ioLiteral('"');
}
/* Read or write a datetime value from channel `f` */
deprecated "'readWriteThis' methods are deprecated. Use 'readThis' and 'writeThis' methods instead."
proc datetime.readWriteThis(f) throws {
if f.writing then writeThis(f);
else readThis(f);
}


Expand Down
Loading

0 comments on commit f6eb173

Please sign in to comment.