-
Notifications
You must be signed in to change notification settings - Fork 419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dyno: implement opaque extern types #22707
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
2473c2c
Add an ExternType type.
DanilaFe 3fdc312
Add type "inference" for opaque extern types.
DanilaFe a011ad7
Add stringify output for ExternType.
DanilaFe 10e84ff
Add tests for the opaque extern types.
DanilaFe 668872d
"handle" converting extern types in convert-uast.
DanilaFe 17a0f92
Remove unused debugger code
DanilaFe 006844f
Add default intents for extern types.
DanilaFe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
/* | ||
* Copyright 2021-2023 Hewlett Packard Enterprise Development LP | ||
* Other additional copyright holders may be indicated within. | ||
* | ||
* The entirety of this work is licensed under the Apache License, | ||
* Version 2.0 (the "License"); you may not use this file except | ||
* in compliance with the License. | ||
* | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
#ifndef CHPL_TYPES_EXTERN_TYPE_H | ||
#define CHPL_TYPES_EXTERN_TYPE_H | ||
|
||
#include "chpl/types/Type.h" | ||
#include "chpl/types/QualifiedType.h" | ||
|
||
namespace chpl { | ||
namespace types { | ||
|
||
class ExternType final : public Type { | ||
private: | ||
UniqueString linkageName_; | ||
|
||
ExternType(UniqueString linkageName) | ||
: Type(typetags::ExternType), linkageName_(linkageName) {} | ||
|
||
bool contentsMatchInner(const Type* other) const override { | ||
const ExternType* rhs = (const ExternType*) other; | ||
return linkageName_ == rhs->linkageName_; | ||
} | ||
|
||
void markUniqueStringsInner(Context* context) const override { | ||
linkageName_.mark(context); | ||
} | ||
|
||
Genericity genericity() const override { | ||
return CONCRETE; | ||
} | ||
|
||
static const owned<ExternType>& getExternType(Context* context, | ||
UniqueString linkageName); | ||
|
||
public: | ||
virtual void stringify(std::ostream& ss, | ||
chpl::StringifyKind stringKind) const override; | ||
|
||
inline UniqueString linkageName() const { return linkageName_; } | ||
|
||
static const ExternType* get(Context* context, UniqueString linkageName); | ||
}; | ||
|
||
} // end namespace types | ||
} // end namespace chpl | ||
|
||
#endif |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
#include "chpl/types/ExternType.h" | ||
#include "chpl/framework/query-impl.h" | ||
|
||
namespace chpl { | ||
namespace types { | ||
|
||
const owned<ExternType>& ExternType::getExternType(Context* context, | ||
UniqueString name) { | ||
QUERY_BEGIN(getExternType, context, name); | ||
auto result = toOwned(new ExternType(name)); | ||
return QUERY_END(result); | ||
} | ||
|
||
void ExternType::stringify(std::ostream& ss, | ||
chpl::StringifyKind stringKind) const { | ||
ss << "extern type "; | ||
linkageName().stringify(ss, stringKind); | ||
} | ||
|
||
const ExternType* ExternType::get(Context* context, UniqueString linkageName) { | ||
return getExternType(context, linkageName).get(); | ||
} | ||
|
||
} // end namespace types | ||
} // end namespace chpl |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we need to preserve both the linkage name and the Chapel name. If a user writes:
Then I believe it should print "helper" (and does in production, fwiw).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a test that checks the stringify'd name of extern types would also be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed this out of band, and Ben and I agreed that we can merge the PR as-is for the time being, and figure out how to keep the original name as part of the type later.
We don't want to show linkage names to the user instead of the Chapel type name, so this feature is important in the long term.
The reason this doesn't immediately work is that our type comparisons in
canPass
are based on pointer equality; however, if we were to store the ID or name of the variable that declared an opaque type in addition to the linkage name, two variables with distinct Chapel names but identical linkage names would not be the exact same type, and thus become incompatible. We don't want that. To address the issue, we probably want to do a deep comparison on types.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also tracked here: #22711