================
@@ -0,0 +1,67 @@
+//===----- 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 declares semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_SEMA_SEMAOPENACC_H
+#define LLVM_CLANG_SEMA_SEMAOPENACC_H
+
+#include "clang/AST/DeclGroup.h"
+#include "clang/Basic/OpenACCKinds.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Sema/Ownership.h"
+
+namespace clang {
+
+class Sema;
----------------
Endilll wrote:

> (Again, nitpicking because this is a precedent)

Your feedback is much appreciated! No worries about nitpicking, as we indeed 
establish a precedent here.

> In which case, I think we should #include "Sema.h" instead of a forward 
> declaration. Forward declarations should be used in the cases where they 
> matter, but otherwise humans waste effort trying to preserve this unneeded 
> property.
>
> Also, #including "Sema.h" makes it clear that the reverse inclusion is not 
> allowed for layering reasons, which is much more important. I can imagine 
> these being introduced by someone wanting to write inline functions in Sema.h 
> that cross component boundaries.

While it would be nice to have layering in Sema, and inline functions in `Sema` 
have already caused me some headaches, I don't think we can include `Sema.h` in 
`SemaOpenACC.h` (or vise-versa, for that matter), as it defeats one of the 
primary goals of this patch: making dependencies between parts of `Sema` 
visible. If we were to include `Sema.h` in `SemaOpenACC.h`, parts that depend 
on `SemaOpenACC` would implicitly have access to the entirety of `Sema`.

> Do we expect that clients realistically use SemaOpenACC but not 
> Sema::OpenACC()? Do we want to encourage clients to accept/store SemaFoo& and 
> SemaBar& instead of Sema&?
> 
> I think we should accept that at least for now, clients will and should still 
> use Sema as the god-object, and that their benefit is not having to pay the 
> compile time of #include "SemaUnrelated.h" or the human time of reading its 
> contents.
> This is the pattern that client code is being migrated to in this patch, and 
> it's much more amenable to mechanical migration.

I wonder if @erichkeane can give us his insight here, as he has more code 
dealing with OpenACC down the line.

https://github.com/llvm/llvm-project/pull/84184
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to