llvm-project
llvm-project copied to clipboard
Published
20 hours ago •
llvm
llvm
[clang] Factor out OpenACC part of `Sema`
This patch moves OpenACC parts of Sema into a separate class SemaOpenACC that is placed in a separate header Sema/SemaOpenACC.h. This patch is intended to be a model of factoring things out of Sema, so I picked a small OpenACC part.
Goals are the following:
- Split
Semainto manageable parts. - Make dependencies between parts visible.
- Improve Clang development cycle by avoiding recompiling unrelated parts of the compiler.
- Avoid compile-time regressions.
- Avoid notational regressions in the code that uses Sema.
@llvm/pr-subscribers-clang
Author: Vlad Serebrennikov (Endilll)
Changes
This patch moves OpenACC parts of Sema into a separate class SemaOpenACC that is placed in a separate header Sema/SemaOpenACC.h. This patch is intended to be a model of factoring things out of Sema, so I picked a small OpenACC part.
Goals are the following:
- Split
Semainto manageable parts. - Make dependencies between parts visible.
- Improve Clang development cycle by avoiding recompiling unrelated parts of the compiler.
- Avoid compile-time regressions.
- Avoid notational regressions in the code that uses Sema.
Full diff: https://github.com/llvm/llvm-project/pull/84184.diff
6 Files Affected:
- (modified) clang/include/clang/Sema/Sema.h (+24-53)
- (added) clang/include/clang/Sema/SemaOpenACC.h (+68)
- (modified) clang/lib/Parse/ParseOpenACC.cpp (+12-10)
- (modified) clang/lib/Sema/Sema.cpp (+3-1)
- (modified) clang/lib/Sema/SemaOpenACC.cpp (+24-20)
- (modified) clang/lib/Sema/TreeTransform.h (+6-5)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f3d3a57104ee07..932676c52b1f30 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -183,6 +183,7 @@ class Preprocessor;
class PseudoDestructorTypeStorage;
class PseudoObjectExpr;
class QualType;
+class SemaOpenACC;
class StandardConversionSequence;
class Stmt;
class StringLiteral;
@@ -466,9 +467,8 @@ class Sema final {
// 37. Name Lookup for RISC-V Vector Intrinsic (SemaRISCVVectorLookup.cpp)
// 38. CUDA (SemaCUDA.cpp)
// 39. HLSL Constructs (SemaHLSL.cpp)
- // 40. OpenACC Constructs (SemaOpenACC.cpp)
- // 41. OpenMP Directives and Clauses (SemaOpenMP.cpp)
- // 42. SYCL Constructs (SemaSYCL.cpp)
+ // 40. OpenMP Directives and Clauses (SemaOpenMP.cpp)
+ // 41. SYCL Constructs (SemaSYCL.cpp)
/// \name Semantic Analysis
/// Implementations are in Sema.cpp
@@ -1200,6 +1200,27 @@ class Sema final {
//
//
+ /// \name Sema Components
+ /// Parts of Sema
+ ///@{
+
+ // Just in this section, private members are followed by public, because
+ // C++ requires us to create (private) objects before (public) references.
+
+private:
+ std::unique_ptr<SemaOpenACC> OpenACCPtr;
+
+public:
+ SemaOpenACC &OpenACC;
+
+ ///@}
+
+ //
+ //
+ // -------------------------------------------------------------------------
+ //
+ //
+
/// \name C++ Access Control
/// Implementations are in SemaAccess.cpp
///@{
@@ -13309,56 +13330,6 @@ class Sema final {
//
//
- /// \name OpenACC Constructs
- /// Implementations are in SemaOpenACC.cpp
- ///@{
-
-public:
- /// Called after parsing an OpenACC Clause so that it can be checked.
- bool ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
- SourceLocation StartLoc);
-
- /// Called after the construct has been parsed, but clauses haven't been
- /// parsed. This allows us to diagnose not-implemented, as well as set up any
- /// state required for parsing the clauses.
- void ActOnOpenACCConstruct(OpenACCDirectiveKind K, SourceLocation StartLoc);
-
- /// Called after the directive, including its clauses, have been parsed and
- /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
- /// happen before any associated declarations or statements have been parsed.
- /// This function is only called when we are parsing a 'statement' context.
- bool ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
- SourceLocation StartLoc);
-
- /// Called after the directive, including its clauses, have been parsed and
- /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
- /// happen before any associated declarations or statements have been parsed.
- /// This function is only called when we are parsing a 'Decl' context.
- bool ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
- SourceLocation StartLoc);
- /// Called when we encounter an associated statement for our construct, this
- /// should check legality of the statement as it appertains to this Construct.
- StmtResult ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
- StmtResult AssocStmt);
-
- /// Called after the directive has been completely parsed, including the
- /// declaration group or associated statement.
- StmtResult ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
- SourceLocation StartLoc,
- SourceLocation EndLoc,
- StmtResult AssocStmt);
- /// Called after the directive has been completely parsed, including the
- /// declaration group or associated statement.
- DeclGroupRef ActOnEndOpenACCDeclDirective();
-
- ///@}
-
- //
- //
- // -------------------------------------------------------------------------
- //
- //
-
/// \name OpenMP Directives and Clauses
/// Implementations are in SemaOpenMP.cpp
///@{
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
new file mode 100644
index 00000000000000..ae50c5f236cc9f
--- /dev/null
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -0,0 +1,68 @@
+//===----- SemaOpenACC.h - Semantic Analysis for OpenACC constructs -------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file implements semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SEMA_SEMAOPENACC_H
+#define LLVM_CLANG_SEMA_SEMAOPENACC_H
+
+#include "clang/Basic/OpenACCKinds.h"
+#include "clang/Sema/Ownership.h"
+
+namespace clang {
+
+class Sema;
+
+class SemaOpenACC {
+public:
+ Sema &Sema;
+
+ /// Called after parsing an OpenACC Clause so that it can be checked.
+ bool ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc);
+
+ /// Called after the construct has been parsed, but clauses haven't been
+ /// parsed. This allows us to diagnose not-implemented, as well as set up any
+ /// state required for parsing the clauses.
+ void ActOnOpenACCConstruct(OpenACCDirectiveKind K, SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'statement' context.
+ bool ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'Decl' context.
+ bool ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+ /// Called when we encounter an associated statement for our construct, this
+ /// should check legality of the statement as it appertains to this Construct.
+ StmtResult ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
+ StmtResult AssocStmt);
+
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ StmtResult ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc,
+ StmtResult AssocStmt);
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ DeclGroupRef ActOnEndOpenACCDeclDirective();
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_SEMA_SEMAOPENACC_H
\ No newline at end of file
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 50e3c39f60919b..5cfdefa5ed4450 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -14,6 +14,7 @@
#include "clang/Parse/ParseDiagnostic.h"
#include "clang/Parse/Parser.h"
#include "clang/Parse/RAIIObjectsForParser.h"
+#include "clang/Sema/SemaOpenACC.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
@@ -777,7 +778,7 @@ bool Parser::ParseOpenACCClause(OpenACCDirectiveKind DirKind) {
SourceLocation ClauseLoc = ConsumeToken();
bool Result = ParseOpenACCClauseParams(DirKind, Kind);
- getActions().ActOnOpenACCClause(Kind, ClauseLoc);
+ getActions().OpenACC.ActOnOpenACCClause(Kind, ClauseLoc);
return Result;
}
@@ -1151,7 +1152,7 @@ Parser::OpenACCDirectiveParseInfo Parser::ParseOpenACCDirective() {
SourceLocation StartLoc = getCurToken().getLocation();
OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(*this);
- getActions().ActOnOpenACCConstruct(DirKind, StartLoc);
+ getActions().OpenACC.ActOnOpenACCConstruct(DirKind, StartLoc);
// Once we've parsed the construct/directive name, some have additional
// specifiers that need to be taken care of. Atomic has an 'atomic-clause'
@@ -1223,12 +1224,13 @@ Parser::DeclGroupPtrTy Parser::ParseOpenACCDirectiveDecl() {
OpenACCDirectiveParseInfo DirInfo = ParseOpenACCDirective();
- if (getActions().ActOnStartOpenACCDeclDirective(DirInfo.DirKind,
- DirInfo.StartLoc))
+ if (getActions().OpenACC.ActOnStartOpenACCDeclDirective(DirInfo.DirKind,
+ DirInfo.StartLoc))
return nullptr;
// TODO OpenACC: Do whatever decl parsing is required here.
- return DeclGroupPtrTy::make(getActions().ActOnEndOpenACCDeclDirective());
+ return DeclGroupPtrTy::make(
+ getActions().OpenACC.ActOnEndOpenACCDeclDirective());
}
// Parse OpenACC Directive on a Statement.
@@ -1239,8 +1241,8 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
ConsumeAnnotationToken();
OpenACCDirectiveParseInfo DirInfo = ParseOpenACCDirective();
- if (getActions().ActOnStartOpenACCStmtDirective(DirInfo.DirKind,
- DirInfo.StartLoc))
+ if (getActions().OpenACC.ActOnStartOpenACCStmtDirective(DirInfo.DirKind,
+ DirInfo.StartLoc))
return StmtError();
StmtResult AssocStmt;
@@ -1249,10 +1251,10 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
ParsingOpenACCDirectiveRAII DirScope(*this, /*Value=*/false);
ParseScope ACCScope(this, getOpenACCScopeFlags(DirInfo.DirKind));
- AssocStmt = getActions().ActOnOpenACCAssociatedStmt(DirInfo.DirKind,
- ParseStatement());
+ AssocStmt = getActions().OpenACC.ActOnOpenACCAssociatedStmt(
+ DirInfo.DirKind, ParseStatement());
}
- return getActions().ActOnEndOpenACCStmtDirective(
+ return getActions().OpenACC.ActOnEndOpenACCStmtDirective(
DirInfo.DirKind, DirInfo.StartLoc, DirInfo.EndLoc, AssocStmt);
}
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 720d5fd5f0428d..d8d7786eff6bb4 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -43,6 +43,7 @@
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaConsumer.h"
#include "clang/Sema/SemaInternal.h"
+#include "clang/Sema/SemaOpenACC.h"
#include "clang/Sema/TemplateDeduction.h"
#include "clang/Sema/TemplateInstCallback.h"
#include "clang/Sema/TypoCorrection.h"
@@ -195,7 +196,8 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
ThreadSafetyDeclCache(nullptr), LateTemplateParser(nullptr),
LateTemplateParserCleanup(nullptr), OpaqueParser(nullptr),
CurContext(nullptr), ExternalSource(nullptr), CurScope(nullptr),
- Ident_super(nullptr),
+ Ident_super(nullptr), OpenACCPtr(new SemaOpenACC{*this}),
+ OpenACC(*OpenACCPtr),
MSPointerToMemberRepresentationMethod(
LangOpts.getMSPointerToMemberRepresentationMethod()),
MSStructPragmaOn(false), VtorDispStack(LangOpts.getVtorDispMode()),
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index d3a602d1c382fa..4ccc379da3085e 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -11,6 +11,8 @@
///
//===----------------------------------------------------------------------===//
+#include "clang/Sema/SemaOpenACC.h"
+
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/OpenACCKinds.h"
#include "clang/Sema/Sema.h"
@@ -18,7 +20,7 @@
using namespace clang;
namespace {
-bool diagnoseConstructAppertainment(Sema &S, OpenACCDirectiveKind K,
+bool diagnoseConstructAppertainment(SemaOpenACC &S, OpenACCDirectiveKind K,
SourceLocation StartLoc, bool IsStmt) {
switch (K) {
default:
@@ -30,25 +32,25 @@ bool diagnoseConstructAppertainment(Sema &S, OpenACCDirectiveKind K,
case OpenACCDirectiveKind::Serial:
case OpenACCDirectiveKind::Kernels:
if (!IsStmt)
- return S.Diag(StartLoc, diag::err_acc_construct_appertainment) << K;
+ return S.Sema.Diag(StartLoc, diag::err_acc_construct_appertainment) << K;
break;
}
return false;
}
} // namespace
-bool Sema::ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
- SourceLocation StartLoc) {
+bool SemaOpenACC::ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc) {
if (ClauseKind == OpenACCClauseKind::Invalid)
return false;
// For now just diagnose that it is unsupported and leave the parsing to do
// whatever it can do. This function will eventually need to start returning
// some sort of Clause AST type, but for now just return true/false based on
// success.
- return Diag(StartLoc, diag::warn_acc_clause_unimplemented) << ClauseKind;
+ return Sema.Diag(StartLoc, diag::warn_acc_clause_unimplemented) << ClauseKind;
}
-void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
- SourceLocation StartLoc) {
+void SemaOpenACC::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
+ SourceLocation StartLoc) {
switch (K) {
case OpenACCDirectiveKind::Invalid:
// Nothing to do here, an invalid kind has nothing we can check here. We
@@ -63,20 +65,20 @@ void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
// here as these constructs do not take any arguments.
break;
default:
- Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+ Sema.Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
break;
}
}
-bool Sema::ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
- SourceLocation StartLoc) {
+bool SemaOpenACC::ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc) {
return diagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/true);
}
-StmtResult Sema::ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
- SourceLocation StartLoc,
- SourceLocation EndLoc,
- StmtResult AssocStmt) {
+StmtResult SemaOpenACC::ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc,
+ StmtResult AssocStmt) {
switch (K) {
default:
return StmtEmpty();
@@ -86,14 +88,14 @@ StmtResult Sema::ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
case OpenACCDirectiveKind::Serial:
case OpenACCDirectiveKind::Kernels:
return OpenACCComputeConstruct::Create(
- getASTContext(), K, StartLoc, EndLoc,
+ Sema.getASTContext(), K, StartLoc, EndLoc,
AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
}
llvm_unreachable("Unhandled case in directive handling?");
}
-StmtResult Sema::ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
- StmtResult AssocStmt) {
+StmtResult SemaOpenACC::ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
+ StmtResult AssocStmt) {
switch (K) {
default:
llvm_unreachable("Unimplemented associated statement application");
@@ -114,9 +116,11 @@ StmtResult Sema::ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
llvm_unreachable("Invalid associated statement application");
}
-bool Sema::ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
- SourceLocation StartLoc) {
+bool SemaOpenACC::ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc) {
return diagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/false);
}
-DeclGroupRef Sema::ActOnEndOpenACCDeclDirective() { return DeclGroupRef{}; }
+DeclGroupRef SemaOpenACC::ActOnEndOpenACCDeclDirective() {
+ return DeclGroupRef{};
+}
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..33642c6edfd60f 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -39,6 +39,7 @@
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaDiagnostic.h"
#include "clang/Sema/SemaInternal.h"
+#include "clang/Sema/SemaOpenACC.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/ErrorHandling.h"
#include <algorithm>
@@ -4000,16 +4001,16 @@ class TreeTransform {
SourceLocation BeginLoc,
SourceLocation EndLoc,
StmtResult StrBlock) {
- getSema().ActOnOpenACCConstruct(K, BeginLoc);
+ getSema().OpenACC.ActOnOpenACCConstruct(K, BeginLoc);
// TODO OpenACC: Include clauses.
- if (getSema().ActOnStartOpenACCStmtDirective(K, BeginLoc))
+ if (getSema().OpenACC.ActOnStartOpenACCStmtDirective(K, BeginLoc))
return StmtError();
- StrBlock = getSema().ActOnOpenACCAssociatedStmt(K, StrBlock);
+ StrBlock = getSema().OpenACC.ActOnOpenACCAssociatedStmt(K, StrBlock);
- return getSema().ActOnEndOpenACCStmtDirective(K, BeginLoc, EndLoc,
- StrBlock);
+ return getSema().OpenACC.ActOnEndOpenACCStmtDirective(K, BeginLoc, EndLoc,
+ StrBlock);
}
private:
Thank you for this factoring! Personally, I think this is a good model to go with for factoring functionality out of Sema and adding a tiny bit of layering to this part of the compiler (full disclosure: Vlad and I worked on this design offline). However, I added several other folks from the community to make sure there's some wider agreement on the approach as a general model.
The basic idea is to split mostly self-contained functionality off into their own classes to reduce the size of Sema.h, have better organization of concerns, make it more explicit where there are semantic connections between language technologies (e.g., where HLSL uses ObjC functionality, etc), and hopefully to help reduce incremental compile times for the project.
Physical separation of parts of Sema while improving incremental compile times means we have to rely on forward declarations, which lead to additional level of indirection at runtime in the form of Sema containing pointers to its components, and components containing a reference back into Sema.
Here's an implementation of an getter function for a Sema component:
SemaOpenACC &OpenACC() {
assert(OpenACCPtr);
return *OpenACCPtr;
}
In order to test compile-time performance implications of introducing a level of indirection, I put together a prototype that moves a dozen of name lookup routines that are on a hot code paths into SemaLookup in the same way this patch does for OpenACC routines. Patch is available in my fork. Results are the following:
- Indirection doesn't seem to introduce any noticeable regressions. Almost all benchmarks exhibit <0.1% change _in both directions-, which I consider noise: https://llvm-compile-time-tracker.com/compare.php?from=e85470232ba2fa49aaee83240741de0bc82a3ffa&to=86d182ab233261563a1ce90c195c9071e76dacc4&stat=instructions:u
- No noticeable regression occur even if we make getter out-of-line on top of the previous test, including stage2 clang build, which is done by stage1 clang built without LTO: https://llvm-compile-time-tracker.com/compare.php?from=86d182ab233261563a1ce90c195c9071e76dacc4&to=6510e5d4cfa7104b89fdf096dda1c8a6d26c044b&stat=instructions:u
My conclusion is that this refactoring is unlikely to introduce regressions for compile-time performance.
This direction looks good to me, mostly I just have nits.
The one major change I'd consider before landing is to find a way to avoid the verbosity regression cor3ntin mentions for
Diag()and friends. Notational regressions within Sema itself may be as important as those of its clients, as it's so much code.
I'd be in favor of just using a 'base' class for these and adding a few of the 'common' ones (Diag, ASTContext, LangOpts), and adding others if we found the need. SourceManager isn't really used enough in these sorts of things to be valuable, nor is PP or the FP Features. The OpenCL features might be worth adding, but only to the OpenCL breakout.
:warning: C/C++ code formatter, clang-format found issues in your code. :warning:
You can test this locally with the following command:
git-clang-format --diff 41afef9066eec8daf517ac357a628cdf30c95e39 b3e2cc20afecf4f2adae043fa3fa51c4e156f657 -- clang/include/clang/Sema/SemaOpenACC.h clang/include/clang/Sema/Sema.h clang/lib/Parse/ParseOpenACC.cpp clang/lib/Sema/JumpDiagnostics.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaOpenACC.cpp clang/lib/Sema/TreeTransform.h
View the diff from clang-format here.
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 2ac994cac7..a3dece5f92 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -11,8 +11,8 @@
///
//===----------------------------------------------------------------------===//
-#include "clang/AST/StmtOpenACC.h"
#include "clang/Sema/SemaOpenACC.h"
+#include "clang/AST/StmtOpenACC.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Sema/Sema.h"
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
This direction looks good to me, mostly I just have nits. The one major change I'd consider before landing is to find a way to avoid the verbosity regression cor3ntin mentions for
Diag()and friends. Notational regressions within Sema itself may be as important as those of its clients, as it's so much code.I'd be in favor of just using a 'base' class for these and adding a few of the 'common' ones (
Diag,ASTContext,LangOpts), and adding others if we found the need.SourceManagerisn't really used enough in these sorts of things to be valuable, nor is PP or the FP Features. The OpenCL features might be worth adding, but only to the OpenCL breakout.
Agreed!
:white_check_mark: With the latest revision this PR passed the Python code formatter.
I added convenience functions to access ASTContext and other widely used facilities of Sema to SemaOpenACC. I intentionally didn't go full base class approach, saving this option for the future.