Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

Commit

Permalink
[clang][OpenMP] Revert "OMPFlushClause is synthetic, no such clause e…
Browse files Browse the repository at this point in the history
…xists"

Summary:
This reverts rL352390 / D57280.

As discussed in https://reviews.llvm.org/D57112#inline-506781,
'flush' clause does not exist in the OpenMP spec, it can not be
specified, and `OMPFlushClause` class is just a helper class.

Now, here's the caveat. I have read @ABataev's
> Well, I think it would be good to filter out OMPC_flush somehow
> because there is no such clause actually, it is a pseudo clause
> for better handling of the flush directive.
as if that clause is pseudo clause that only exists for the sole
purpose of simplifying the parser. As in, it never reaches AST.

I did not however try to verify that. Too bad, i was wrong.
It absolutely *does* reach AST. Therefore my understanding/justification
for the change was flawed, which makes the patch a regression which **must** be reverted.

@gribozavr has brought that up again in https://reviews.llvm.org/D57112#inline-521238

> > ...
> Sorry to be late for this discussion, but I don't think this conclusion
> follows. ASTMatchers are supposed to match the AST as it is.
> Even if OMPC_flush is synthetic, it exists in the AST, and users might
> want to match it. I think users would find anything else (trying to filter
> out AST nodes that are not in the source code) to be surprising. For example,
> there's a matcher materializeTemporaryExpr even though this AST node is a
> Clang invention and is not a part of the C++ spec.
>
> Matching only constructs that appear in the source code is not feasible with
> ASTMatchers, because they are based on Clang's AST that exposes tons of semantic
> information, and its design is dictated by the structure of the semantic information.
> See "RFC: Tree-based refactorings with Clang" in cfe-dev for a library that will
> focus on representing source code as faithfully as possible.
>
> Not to even mention that this code is in ASTTypeTraits, a general library for
> handling AST nodes, not specifically for AST Matchers...

Reviewers: gribozavr, ABataev, rjmccall, aaron.ballman

Reviewed By: gribozavr, ABataev

Subscribers: dylanmckay, guansong, arphaman, jdoerfert, cfe-commits, gribozavr, ABataev

Tags: #clang, #openmp

Differential Revision: https://reviews.llvm.org/D58979

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@355486 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
LebedevRI committed Mar 6, 2019
1 parent 38f1a52 commit 134d9aa
Show file tree
Hide file tree
Showing 11 changed files with 1 addition and 15 deletions.
3 changes: 0 additions & 3 deletions include/clang/AST/OpenMPClause.h
Original file line number Diff line number Diff line change
Expand Up @@ -5421,7 +5421,6 @@ class OMPClauseVisitorBase {

#define OPENMP_CLAUSE(Name, Class) \
RetTy Visit ## Class (PTR(Class) S) { DISPATCH(Class); }
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"

RetTy Visit(PTR(OMPClause) S) {
Expand All @@ -5430,7 +5429,6 @@ class OMPClauseVisitorBase {
default: llvm_unreachable("Unknown clause kind!");
#define OPENMP_CLAUSE(Name, Class) \
case OMPC_ ## Name : return Visit ## Class(static_cast<PTR(Class)>(S));
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
}
}
Expand Down Expand Up @@ -5462,7 +5460,6 @@ class OMPClausePrinter final : public OMPClauseVisitor<OMPClausePrinter> {
: OS(OS), Policy(Policy) {}

#define OPENMP_CLAUSE(Name, Class) void Visit##Class(Class *S);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
};

Expand Down
2 changes: 0 additions & 2 deletions include/clang/AST/RecursiveASTVisitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,6 @@ template <typename Derived> class RecursiveASTVisitor {
bool TraverseOMPLoopDirective(OMPLoopDirective *S);
bool TraverseOMPClause(OMPClause *C);
#define OPENMP_CLAUSE(Name, Class) bool Visit##Class(Class *C);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
/// Process clauses with list of variables.
template <typename T> bool VisitOMPClauseList(T *Node);
Expand Down Expand Up @@ -2796,7 +2795,6 @@ bool RecursiveASTVisitor<Derived>::TraverseOMPClause(OMPClause *C) {
case OMPC_##Name: \
TRY_TO(Visit##Class(static_cast<Class *>(C))); \
break;
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
case OMPC_threadprivate:
case OMPC_uniform:
Expand Down
1 change: 1 addition & 0 deletions include/clang/Basic/OpenMPKinds.def
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ OPENMP_CLAUSE(ordered, OMPOrderedClause)
OPENMP_CLAUSE(nowait, OMPNowaitClause)
OPENMP_CLAUSE(untied, OMPUntiedClause)
OPENMP_CLAUSE(mergeable, OMPMergeableClause)
OPENMP_CLAUSE(flush, OMPFlushClause)
OPENMP_CLAUSE(read, OMPReadClause)
OPENMP_CLAUSE(write, OMPWriteClause)
OPENMP_CLAUSE(update, OMPUpdateClause)
Expand Down
1 change: 0 additions & 1 deletion include/clang/Basic/OpenMPKinds.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ enum OpenMPClauseKind {
#define OPENMP_CLAUSE(Name, Class) \
OMPC_##Name,
#include "clang/Basic/OpenMPKinds.def"
OMPC_flush,
OMPC_threadprivate,
OMPC_uniform,
OMPC_unknown
Expand Down
1 change: 0 additions & 1 deletion include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -2686,7 +2686,6 @@ class OMPClauseReader : public OMPClauseVisitor<OMPClauseReader> {
: Record(Record), Context(Record.getContext()) {}

#define OPENMP_CLAUSE(Name, Class) void Visit##Class(Class *C);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
OMPClause *readClause();
void VisitOMPClauseWithPreInit(OMPClauseWithPreInit *C);
Expand Down
1 change: 0 additions & 1 deletion include/clang/Serialization/ASTWriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,6 @@ class OMPClauseWriter : public OMPClauseVisitor<OMPClauseWriter> {
public:
OMPClauseWriter(ASTRecordWriter &Record) : Record(Record) {}
#define OPENMP_CLAUSE(Name, Class) void Visit##Class(Class *S);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
void writeClause(OMPClause *C);
void VisitOMPClauseWithPreInit(OMPClauseWithPreInit *C);
Expand Down
1 change: 0 additions & 1 deletion lib/AST/OpenMPClause.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ OMPClause::child_range OMPClause::children() {
#define OPENMP_CLAUSE(Name, Class) \
case OMPC_##Name: \
return static_cast<Class *>(this)->children();
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
}
llvm_unreachable("unknown OMPClause");
Expand Down
1 change: 0 additions & 1 deletion lib/AST/StmtProfile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,6 @@ class OMPClauseProfiler : public ConstOMPClauseVisitor<OMPClauseProfiler> {
OMPClauseProfiler(StmtProfiler *P) : Profiler(P) { }
#define OPENMP_CLAUSE(Name, Class) \
void Visit##Class(const Class *C);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
void VistOMPClauseWithPreInit(const OMPClauseWithPreInit *C);
void VistOMPClauseWithPostUpdate(const OMPClauseWithPostUpdate *C);
Expand Down
2 changes: 0 additions & 2 deletions lib/Basic/OpenMPKinds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ const char *clang::getOpenMPClauseName(OpenMPClauseKind Kind) {
case OMPC_##Name: \
return #Name;
#include "clang/Basic/OpenMPKinds.def"
case OMPC_flush:
return "flush";
case OMPC_uniform:
return "uniform";
case OMPC_threadprivate:
Expand Down
2 changes: 0 additions & 2 deletions lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,6 @@ class TreeTransform {
#define OPENMP_CLAUSE(Name, Class) \
LLVM_ATTRIBUTE_NOINLINE \
OMPClause *Transform ## Class(Class *S);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"

/// Build a new qualified type given its unqualified type and type location.
Expand Down Expand Up @@ -3323,7 +3322,6 @@ OMPClause *TreeTransform<Derived>::TransformOMPClause(OMPClause *S) {
#define OPENMP_CLAUSE(Name, Class) \
case OMPC_ ## Name : \
return getDerived().Transform ## Class(cast<Class>(S));
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
}

Expand Down
1 change: 0 additions & 1 deletion tools/libclang/CIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2130,7 +2130,6 @@ class OMPClauseEnqueue : public ConstOMPClauseVisitor<OMPClauseEnqueue> {
OMPClauseEnqueue(EnqueueVisitor *Visitor) : Visitor(Visitor) { }
#define OPENMP_CLAUSE(Name, Class) \
void Visit##Class(const Class *C);
OPENMP_CLAUSE(flush, OMPFlushClause)
#include "clang/Basic/OpenMPKinds.def"
void VisitOMPClauseWithPreInit(const OMPClauseWithPreInit *C);
void VisitOMPClauseWithPostUpdate(const OMPClauseWithPostUpdate *C);
Expand Down

0 comments on commit 134d9aa

Please sign in to comment.