Skip to content

[flang][OpenMP] Issue a warning when parsing future directive spelling #147765

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

Merged
merged 8 commits into from
Jul 10, 2025
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion flang/include/flang/Parser/parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4612,7 +4612,7 @@ struct OmpDirectiveSpecification {

struct OmpMetadirectiveDirective {
TUPLE_CLASS_BOILERPLATE(OmpMetadirectiveDirective);
std::tuple<OmpClauseList> t;
std::tuple<Verbatim, OmpClauseList> t;
CharBlock source;
};

Expand Down
78 changes: 51 additions & 27 deletions flang/lib/Parser/openmp-parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Frontend/OpenMP/OMP.h"

// OpenMP Directives and Clauses
Expand Down Expand Up @@ -104,8 +105,13 @@ struct OmpDirectiveNameParser {
using Token = TokenStringMatch<false, false>;

std::optional<resultType> Parse(ParseState &state) const {
if (state.BytesRemaining() == 0) {
return std::nullopt;
}
auto begin{state.GetLocation()};
for (const NameWithId &nid : directives()) {
char next{static_cast<char>(std::tolower(*begin))};

for (const NameWithId &nid : directives_starting_with(next)) {
if (attempt(Token(nid.first.data())).Parse(state)) {
OmpDirectiveName n;
n.v = nid.second;
Expand All @@ -118,30 +124,46 @@ struct OmpDirectiveNameParser {

private:
using NameWithId = std::pair<std::string, llvm::omp::Directive>;
using ConstIterator = std::vector<NameWithId>::const_iterator;

llvm::iterator_range<const NameWithId *> directives() const;
void initTokens(NameWithId *) const;
llvm::iterator_range<ConstIterator> directives_starting_with(
char initial) const;
void initTokens(std::vector<NameWithId>[]) const;
};

llvm::iterator_range<const OmpDirectiveNameParser::NameWithId *>
OmpDirectiveNameParser::directives() const {
static NameWithId table[llvm::omp::Directive_enumSize];
llvm::iterator_range<OmpDirectiveNameParser::ConstIterator>
OmpDirectiveNameParser::directives_starting_with(char initial) const {
static const std::vector<NameWithId> empty{};
if (initial < 'a' || initial > 'z') {
return llvm::make_range(std::cbegin(empty), std::cend(empty));
}

static std::vector<NameWithId> table['z' - 'a' + 1];
[[maybe_unused]] static bool init = (initTokens(table), true);
return llvm::make_range(std::cbegin(table), std::cend(table));

int index = initial - 'a';
return llvm::make_range(std::cbegin(table[index]), std::cend(table[index]));
}

void OmpDirectiveNameParser::initTokens(NameWithId *table) const {
void OmpDirectiveNameParser::initTokens(std::vector<NameWithId> table[]) const {
for (size_t i{0}, e{llvm::omp::Directive_enumSize}; i != e; ++i) {
llvm::StringSet spellings;
auto id{static_cast<llvm::omp::Directive>(i)};
llvm::StringRef name{
llvm::omp::getOpenMPDirectiveName(id, llvm::omp::FallbackVersion)};
table[i] = std::make_pair(name.str(), id);
for (unsigned version : llvm::omp::getOpenMPVersions()) {
spellings.insert(llvm::omp::getOpenMPDirectiveName(id, version));
}
for (auto &[name, _] : spellings) {
char initial{static_cast<char>(std::tolower(name.front()))};
table[initial - 'a'].emplace_back(name.str(), id);
}
}
// Sort the table with respect to the directive name length in a descending
// order. This is to make sure that longer names are tried first, before
// any potential prefix (e.g. "target update" before "target").
std::sort(table, table + llvm::omp::Directive_enumSize,
[](auto &a, auto &b) { return a.first.size() > b.first.size(); });
for (int initial{'a'}; initial != 'z' + 1; ++initial) {
llvm::stable_sort(table[initial - 'a'],
[](auto &a, auto &b) { return a.first.size() > b.first.size(); });
}
}

// --- Modifier helpers -----------------------------------------------
Expand Down Expand Up @@ -1172,7 +1194,7 @@ TYPE_PARSER(sourced(construct<OpenMPUtilityConstruct>(
sourced(Parser<OmpNothingDirective>{}))))))

TYPE_PARSER(sourced(construct<OmpMetadirectiveDirective>(
"METADIRECTIVE" >> Parser<OmpClauseList>{})))
verbatim("METADIRECTIVE"_tok), Parser<OmpClauseList>{})))

// Omp directives enclosing do loop
TYPE_PARSER(sourced(construct<OmpLoopDirective>(first(
Expand Down Expand Up @@ -1478,7 +1500,7 @@ TYPE_PARSER(
// In this context "TARGET UPDATE" can be parsed as a TARGET directive
// followed by an UPDATE clause. This is the only combination at the
// moment, exclude it explicitly.
(!"TARGET UPDATE"_sptok) >=
(!("TARGET UPDATE"_sptok || "TARGET_UPDATE"_sptok)) >=
construct<OmpBlockDirective>(first(
"MASKED" >> pure(llvm::omp::Directive::OMPD_masked),
"MASTER" >> pure(llvm::omp::Directive::OMPD_master),
Expand All @@ -1491,6 +1513,7 @@ TYPE_PARSER(
"SCOPE" >> pure(llvm::omp::Directive::OMPD_scope),
"SINGLE" >> pure(llvm::omp::Directive::OMPD_single),
"TARGET DATA" >> pure(llvm::omp::Directive::OMPD_target_data),
"TARGET_DATA" >> pure(llvm::omp::Directive::OMPD_target_data),
"TARGET PARALLEL" >> pure(llvm::omp::Directive::OMPD_target_parallel),
"TARGET TEAMS" >> pure(llvm::omp::Directive::OMPD_target_teams),
"TARGET" >> pure(llvm::omp::Directive::OMPD_target),
Expand All @@ -1510,13 +1533,13 @@ TYPE_PARSER(construct<OmpInitializerClause>(
construct<OmpInitializerClause>(Parser<OmpInitializerProc>{})))

// OpenMP 5.2: 7.5.4 Declare Variant directive
TYPE_PARSER(sourced(
construct<OmpDeclareVariantDirective>(verbatim("DECLARE VARIANT"_tok),
"(" >> maybe(name / ":"), name / ")", Parser<OmpClauseList>{})))
TYPE_PARSER(sourced(construct<OmpDeclareVariantDirective>(
verbatim("DECLARE VARIANT"_tok) || verbatim("DECLARE_VARIANT"_tok),
"(" >> maybe(name / ":"), name / ")", Parser<OmpClauseList>{})))

// 2.16 Declare Reduction Construct
TYPE_PARSER(sourced(construct<OpenMPDeclareReductionConstruct>(
verbatim("DECLARE REDUCTION"_tok),
verbatim("DECLARE REDUCTION"_tok) || verbatim("DECLARE_REDUCTION"_tok),
"(" >> indirect(Parser<OmpReductionSpecifier>{}) / ")",
maybe(Parser<OmpClauseList>{}))))

Expand All @@ -1535,7 +1558,8 @@ TYPE_PARSER(

// 2.10.6 Declare Target Construct
TYPE_PARSER(sourced(construct<OpenMPDeclareTargetConstruct>(
verbatim("DECLARE TARGET"_tok), Parser<OmpDeclareTargetSpecifier>{})))
verbatim("DECLARE TARGET"_tok) || verbatim("DECLARE_TARGET"_tok),
Parser<OmpDeclareTargetSpecifier>{})))

static OmpMapperSpecifier ConstructOmpMapperSpecifier(
std::optional<Name> &&mapperName, TypeSpec &&typeSpec, Name &&varName) {
Expand All @@ -1562,9 +1586,9 @@ TYPE_PARSER(applyFunction<OmpMapperSpecifier>(ConstructOmpMapperSpecifier,
maybe(name / ":" / !":"_tok), typeSpec / "::", name))

// OpenMP 5.2: 5.8.8 Declare Mapper Construct
TYPE_PARSER(sourced(
construct<OpenMPDeclareMapperConstruct>(verbatim("DECLARE MAPPER"_tok),
parenthesized(Parser<OmpMapperSpecifier>{}), Parser<OmpClauseList>{})))
TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok),
parenthesized(Parser<OmpMapperSpecifier>{}), Parser<OmpClauseList>{})))

TYPE_PARSER(construct<OmpReductionCombiner>(Parser<AssignmentStmt>{}) ||
construct<OmpReductionCombiner>(Parser<FunctionReference>{}))
Expand Down Expand Up @@ -1609,9 +1633,9 @@ TYPE_PARSER(sourced(construct<OpenMPAllocatorsConstruct>(
TYPE_PARSER(construct<OmpEndAllocators>(startOmpLine >> "END ALLOCATORS"_tok))

// 2.8.2 Declare Simd construct
TYPE_PARSER(
sourced(construct<OpenMPDeclareSimdConstruct>(verbatim("DECLARE SIMD"_tok),
maybe(parenthesized(name)), Parser<OmpClauseList>{})))
TYPE_PARSER(sourced(construct<OpenMPDeclareSimdConstruct>(
verbatim("DECLARE SIMD"_tok) || verbatim("DECLARE_SIMD"_tok),
maybe(parenthesized(name)), Parser<OmpClauseList>{})))

// 2.4 Requires construct
TYPE_PARSER(sourced(construct<OpenMPRequiresConstruct>(
Expand Down Expand Up @@ -1663,7 +1687,7 @@ TYPE_PARSER(sourced(construct<OmpAssumeDirective>(
verbatim("ASSUME"_tok), Parser<OmpClauseList>{})))

TYPE_PARSER(sourced(construct<OmpEndAssumeDirective>(
verbatim(startOmpLine >> "END ASSUME"_tok))))
startOmpLine >> verbatim("END ASSUME"_tok))))

TYPE_PARSER(sourced(
construct<OpenMPAssumeConstruct>(Parser<OmpAssumeDirective>{} / endOmpLine,
Expand Down
181 changes: 181 additions & 0 deletions flang/lib/Semantics/check-omp-structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,55 @@ void OmpStructureChecker::CheckVariableListItem(
}
}

void OmpStructureChecker::CheckDirectiveSpelling(
parser::CharBlock spelling, llvm::omp::Directive id) {
// Directive names that contain spaces can be spelled in the source without
// any of the spaces. Because of that getOpenMPKind* is not guaranteed to
// work with the source spelling as the argument.
//
// To verify the source spellings, we have to get the spelling for a given
// version, remove spaces and compare it with the source spelling (also
// with spaces removed).
auto removeSpaces = [](llvm::StringRef s) {
std::string n{s.str()};
for (auto it{n.begin()}; it != n.end();) {
it = isspace(*it) ? n.erase(it) : std::next(it);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit of a nitpick but I think if you iterate backwards it will run slightly faster for strings containing multiple spaces because when it gets to erasing the left-most space it won't have to move left as many characters.
I doubt the effect would make much difference in practice, but it isn't hard to do either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return n;
};

std::string lowerNoWS{removeSpaces(
parser::ToLowerCaseLetters({spelling.begin(), spelling.size()}))};
llvm::StringRef ref(lowerNoWS);
if (ref.starts_with("end")) {
ref = ref.drop_front(3);
}

unsigned version{context_.langOptions().OpenMPVersion};

// For every "future" version v, check if the check if the corresponding
// spelling of id was introduced later than the current version. If so,
// and if that spelling matches the source spelling, issue a warning.
for (unsigned v : llvm::omp::getOpenMPVersions()) {
if (v <= version) {
continue;
}
llvm::StringRef name{llvm::omp::getOpenMPDirectiveName(id, v)};
auto [kind, versions]{llvm::omp::getOpenMPDirectiveKindAndVersions(name)};
assert(kind == id && "Directive kind mismatch");

if (static_cast<int>(version) >= versions.Min) {
continue;
}
if (ref == removeSpaces(name)) {
context_.Say(spelling,
"Directive spelling '%s' is introduced in a later OpenMP version, %s"_warn_en_US,
parser::ToUpperCaseLetters(ref), TryVersion(versions.Min));
break;
}
}
}

void OmpStructureChecker::CheckMultipleOccurrence(
semantics::UnorderedSymbolSet &listVars,
const std::list<parser::Name> &nameList, const parser::CharBlock &item,
Expand Down Expand Up @@ -436,7 +485,133 @@ void OmpStructureChecker::Leave(const parser::OmpDirectiveSpecification &) {
}
}

template <typename Checker> struct DirectiveSpellingVisitor {
using Directive = llvm::omp::Directive;

DirectiveSpellingVisitor(Checker &&checker) : checker_(std::move(checker)) {}

template <typename T> bool Pre(const T &) { return true; }
template <typename T> void Post(const T &) {}

bool Pre(const parser::OmpSectionsDirective &x) {
checker_(x.source, x.v);
return false;
}
bool Pre(const parser::OpenMPDeclarativeAllocate &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_allocate);
return false;
}
bool Pre(const parser::OmpDispatchDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_dispatch);
return false;
}
bool Pre(const parser::OmpErrorDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_error);
return false;
}
bool Pre(const parser::OmpNothingDirective &x) {
checker_(x.source, Directive::OMPD_nothing);
return false;
}
bool Pre(const parser::OpenMPExecutableAllocate &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_allocate);
return false;
}
bool Pre(const parser::OpenMPAllocatorsConstruct &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_allocators);
return false;
}
bool Pre(const parser::OmpAssumeDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assume);
return false;
}
bool Pre(const parser::OmpEndAssumeDirective &x) {
checker_(x.v.source, Directive::OMPD_assume);
return false;
}
bool Pre(const parser::OmpCriticalDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_critical);
return false;
}
bool Pre(const parser::OmpEndCriticalDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_critical);
return false;
}
bool Pre(const parser::OmpMetadirectiveDirective &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_metadirective);
return false;
}
bool Pre(const parser::OpenMPDeclarativeAssumes &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_assumes);
return false;
}
bool Pre(const parser::OpenMPDeclareMapperConstruct &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_mapper);
return false;
}
bool Pre(const parser::OpenMPDeclareReductionConstruct &x) {
checker_(std::get<parser::Verbatim>(x.t).source,
Directive::OMPD_declare_reduction);
return false;
}
bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_simd);
return false;
}
bool Pre(const parser::OpenMPDeclareTargetConstruct &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_declare_target);
return false;
}
bool Pre(const parser::OmpDeclareVariantDirective &x) {
checker_(std::get<parser::Verbatim>(x.t).source,
Directive::OMPD_declare_variant);
return false;
}
bool Pre(const parser::OpenMPThreadprivate &x) {
checker_(
std::get<parser::Verbatim>(x.t).source, Directive::OMPD_threadprivate);
return false;
}
bool Pre(const parser::OpenMPRequiresConstruct &x) {
checker_(std::get<parser::Verbatim>(x.t).source, Directive::OMPD_requires);
return false;
}

bool Pre(const parser::OmpBlockDirective &x) {
checker_(x.source, x.v);
return false;
}

bool Pre(const parser::OmpLoopDirective &x) {
checker_(x.source, x.v);
return false;
}

bool Pre(const parser::OmpDirectiveSpecification &x) {
auto &name = std::get<parser::OmpDirectiveName>(x.t);
checker_(name.source, name.v);
return false;
}

private:
Checker checker_;
};

template <typename T>
DirectiveSpellingVisitor(T &&) -> DirectiveSpellingVisitor<T>;

void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) {
DirectiveSpellingVisitor visitor(
[this](parser::CharBlock source, llvm::omp::Directive id) {
return CheckDirectiveSpelling(source, id);
});
parser::Walk(x, visitor);

// Simd Construct with Ordered Construct Nesting check
// We cannot use CurrentDirectiveIsNested() here because
// PushContextAndClauseSets() has not been called yet, it is
Expand All @@ -461,6 +636,12 @@ void OmpStructureChecker::Leave(const parser::OpenMPConstruct &) {
}

void OmpStructureChecker::Enter(const parser::OpenMPDeclarativeConstruct &x) {
DirectiveSpellingVisitor visitor(
[this](parser::CharBlock source, llvm::omp::Directive id) {
return CheckDirectiveSpelling(source, id);
});
parser::Walk(x, visitor);

EnterDirectiveNest(DeclarativeNest);
}

Expand Down
2 changes: 2 additions & 0 deletions flang/lib/Semantics/check-omp-structure.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ class OmpStructureChecker
private:
bool CheckAllowedClause(llvmOmpClause clause);
void CheckVariableListItem(const SymbolSourceMap &symbols);
void CheckDirectiveSpelling(
parser::CharBlock spelling, llvm::omp::Directive id);
void CheckMultipleOccurrence(semantics::UnorderedSymbolSet &listVars,
const std::list<parser::Name> &nameList, const parser::CharBlock &item,
const std::string &clauseName);
Expand Down
Loading