Skip to content

Commit 1c91d9b

Browse files
authored
[flang] Ensure that portability warnings are conditional (#71857)
Before emitting a warning message, code should check that the usage in question should be diagnosed by calling ShouldWarn(). A fair number of sites in the code do not, and can emit portability warnings unconditionally, which can confuse a user that hasn't asked for them (-pedantic) and isn't terribly concerned about portability *to* other compilers. Add calls to ShouldWarn() or IsEnabled() around messages that need them, and add -pedantic to tests that now require it to test their portability messages, and add more expected message lines to those tests when -pedantic causes other diagnostics to fire.
1 parent a5eb6bd commit 1c91d9b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

78 files changed

+568
-347
lines changed

flang/include/flang/Common/Fortran-features.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,23 @@ ENUM_CLASS(LanguageFeature, BackslashEscapes, OldDebugLines,
3737
DistinguishableSpecifics, DefaultSave, PointerInSeqType, NonCharacterFormat,
3838
SaveMainProgram, SaveBigMainProgramVariables,
3939
DistinctArrayConstructorLengths, PPCVector, RelaxedIntentInChecking,
40-
ForwardRefImplicitNoneData, NullActualForAllocatable)
40+
ForwardRefImplicitNoneData, NullActualForAllocatable,
41+
ActualIntegerConvertedToSmallerKind, HollerithOrCharacterAsBOZ,
42+
BindingAsProcedure, StatementFunctionExtensions,
43+
UseGenericIntrinsicWhenSpecificDoesntMatch, DataStmtExtensions,
44+
RedundantContiguous, InitBlankCommon, EmptyBindCDerivedType,
45+
MiscSourceExtensions, AllocateToOtherLength, LongNames, IntrinsicAsSpecific,
46+
BenignNameClash, BenignRedundancy, NullMoldAllocatableComponentValue,
47+
NopassScalarBase, MiscUseExtensions, ImpliedDoIndexScope,
48+
DistinctCommonSizes)
4149

4250
// Portability and suspicious usage warnings for conforming code
4351
ENUM_CLASS(UsageWarning, Portability, PointerToUndefinable,
4452
NonTargetPassedToTarget, PointerToPossibleNoncontiguous,
4553
ShortCharacterActual, ExprPassedToVolatile, ImplicitInterfaceActual,
4654
PolymorphicTransferArg, PointerComponentTransferArg, TransferSizePresence,
47-
F202XAllocatableBreakingChange)
55+
F202XAllocatableBreakingChange, DimMustBePresent, CommonBlockPadding,
56+
LogicalVsCBool, BindCCharLength)
4857

4958
using LanguageFeatures = EnumSet<LanguageFeature, LanguageFeature_enumSize>;
5059
using UsageWarnings = EnumSet<UsageWarning, UsageWarning_enumSize>;

flang/include/flang/Evaluate/common.h

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef FORTRAN_EVALUATE_COMMON_H_
1010
#define FORTRAN_EVALUATE_COMMON_H_
1111

12+
#include "flang/Common/Fortran-features.h"
1213
#include "flang/Common/Fortran.h"
1314
#include "flang/Common/default-kinds.h"
1415
#include "flang/Common/enum-set.h"
@@ -215,22 +216,27 @@ template <typename A> class Expr;
215216
class FoldingContext {
216217
public:
217218
FoldingContext(const common::IntrinsicTypeDefaultKinds &d,
218-
const IntrinsicProcTable &t, const TargetCharacteristics &c)
219-
: defaults_{d}, intrinsics_{t}, targetCharacteristics_{c} {}
219+
const IntrinsicProcTable &t, const TargetCharacteristics &c,
220+
const common::LanguageFeatureControl &lfc)
221+
: defaults_{d}, intrinsics_{t}, targetCharacteristics_{c},
222+
languageFeatures_{lfc} {}
220223
FoldingContext(const parser::ContextualMessages &m,
221224
const common::IntrinsicTypeDefaultKinds &d, const IntrinsicProcTable &t,
222-
const TargetCharacteristics &c)
223-
: messages_{m}, defaults_{d}, intrinsics_{t}, targetCharacteristics_{c} {}
225+
const TargetCharacteristics &c, const common::LanguageFeatureControl &lfc)
226+
: messages_{m}, defaults_{d}, intrinsics_{t}, targetCharacteristics_{c},
227+
languageFeatures_{lfc} {}
224228
FoldingContext(const FoldingContext &that)
225229
: messages_{that.messages_}, defaults_{that.defaults_},
226230
intrinsics_{that.intrinsics_},
227231
targetCharacteristics_{that.targetCharacteristics_},
228-
pdtInstance_{that.pdtInstance_}, impliedDos_{that.impliedDos_} {}
232+
pdtInstance_{that.pdtInstance_}, impliedDos_{that.impliedDos_},
233+
languageFeatures_{that.languageFeatures_} {}
229234
FoldingContext(
230235
const FoldingContext &that, const parser::ContextualMessages &m)
231236
: messages_{m}, defaults_{that.defaults_}, intrinsics_{that.intrinsics_},
232237
targetCharacteristics_{that.targetCharacteristics_},
233-
pdtInstance_{that.pdtInstance_}, impliedDos_{that.impliedDos_} {}
238+
pdtInstance_{that.pdtInstance_}, impliedDos_{that.impliedDos_},
239+
languageFeatures_{that.languageFeatures_} {}
234240

235241
parser::ContextualMessages &messages() { return messages_; }
236242
const parser::ContextualMessages &messages() const { return messages_; }
@@ -242,6 +248,9 @@ class FoldingContext {
242248
const TargetCharacteristics &targetCharacteristics() const {
243249
return targetCharacteristics_;
244250
}
251+
const common::LanguageFeatureControl &languageFeatures() const {
252+
return languageFeatures_;
253+
}
245254
bool inModuleFile() const { return inModuleFile_; }
246255
FoldingContext &set_inModuleFile(bool yes = true) {
247256
inModuleFile_ = yes;
@@ -272,6 +281,7 @@ class FoldingContext {
272281
const semantics::DerivedTypeSpec *pdtInstance_{nullptr};
273282
bool inModuleFile_{false};
274283
std::map<parser::CharBlock, ConstantSubscript> impliedDos_;
284+
const common::LanguageFeatureControl &languageFeatures_;
275285
};
276286

277287
void RealFlagWarnings(FoldingContext &, const RealFlags &, const char *op);

flang/include/flang/Lower/Bridge.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ class LoweringBridge {
5858
const Fortran::parser::AllCookedSources &allCooked,
5959
llvm::StringRef triple, fir::KindMapping &kindMap,
6060
const Fortran::lower::LoweringOptions &loweringOptions,
61-
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults) {
61+
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
62+
const Fortran::common::LanguageFeatureControl &languageFeatures) {
6263
return LoweringBridge(ctx, semanticsContext, defaultKinds, intrinsics,
6364
targetCharacteristics, allCooked, triple, kindMap,
64-
loweringOptions, envDefaults);
65+
loweringOptions, envDefaults, languageFeatures);
6566
}
6667

6768
//===--------------------------------------------------------------------===//
@@ -99,6 +100,10 @@ class LoweringBridge {
99100
return envDefaults;
100101
}
101102

103+
const Fortran::common::LanguageFeatureControl &getLanguageFeatures() const {
104+
return languageFeatures;
105+
}
106+
102107
/// Create a folding context. Careful: this is very expensive.
103108
Fortran::evaluate::FoldingContext createFoldingContext() const;
104109

@@ -132,7 +137,8 @@ class LoweringBridge {
132137
const Fortran::parser::AllCookedSources &cooked, llvm::StringRef triple,
133138
fir::KindMapping &kindMap,
134139
const Fortran::lower::LoweringOptions &loweringOptions,
135-
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults);
140+
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
141+
const Fortran::common::LanguageFeatureControl &languageFeatures);
136142
LoweringBridge() = delete;
137143
LoweringBridge(const LoweringBridge &) = delete;
138144

@@ -147,6 +153,7 @@ class LoweringBridge {
147153
fir::KindMapping &kindMap;
148154
const Fortran::lower::LoweringOptions &loweringOptions;
149155
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults;
156+
const Fortran::common::LanguageFeatureControl &languageFeatures;
150157
};
151158

152159
} // namespace lower

flang/lib/Evaluate/check-expression.cpp

Lines changed: 54 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "flang/Evaluate/tools.h"
1313
#include "flang/Evaluate/traverse.h"
1414
#include "flang/Evaluate/type.h"
15+
#include "flang/Semantics/semantics.h"
1516
#include "flang/Semantics/symbol.h"
1617
#include "flang/Semantics/tools.h"
1718
#include <set>
@@ -1030,23 +1031,46 @@ class StmtFunctionChecker
10301031
using Result = std::optional<parser::Message>;
10311032
using Base = AnyTraverse<StmtFunctionChecker, Result>;
10321033
StmtFunctionChecker(const Symbol &sf, FoldingContext &context)
1033-
: Base{*this}, sf_{sf}, context_{context} {}
1034+
: Base{*this}, sf_{sf}, context_{context} {
1035+
if (!context_.languageFeatures().IsEnabled(
1036+
common::LanguageFeature::StatementFunctionExtensions)) {
1037+
severity_ = parser::Severity::Error;
1038+
} else if (context_.languageFeatures().ShouldWarn(
1039+
common::LanguageFeature::StatementFunctionExtensions)) {
1040+
severity_ = parser::Severity::Portability;
1041+
}
1042+
}
10341043
using Base::operator();
10351044

10361045
template <typename T> Result operator()(const ArrayConstructor<T> &) const {
1037-
return parser::Message{sf_.name(),
1038-
"Statement function '%s' should not contain an array constructor"_port_en_US,
1039-
sf_.name()};
1046+
if (severity_) {
1047+
auto msg{
1048+
"Statement function '%s' should not contain an array constructor"_port_en_US};
1049+
msg.set_severity(*severity_);
1050+
return parser::Message{sf_.name(), std::move(msg), sf_.name()};
1051+
} else {
1052+
return std::nullopt;
1053+
}
10401054
}
10411055
Result operator()(const StructureConstructor &) const {
1042-
return parser::Message{sf_.name(),
1043-
"Statement function '%s' should not contain a structure constructor"_port_en_US,
1044-
sf_.name()};
1056+
if (severity_) {
1057+
auto msg{
1058+
"Statement function '%s' should not contain a structure constructor"_port_en_US};
1059+
msg.set_severity(*severity_);
1060+
return parser::Message{sf_.name(), std::move(msg), sf_.name()};
1061+
} else {
1062+
return std::nullopt;
1063+
}
10451064
}
10461065
Result operator()(const TypeParamInquiry &) const {
1047-
return parser::Message{sf_.name(),
1048-
"Statement function '%s' should not contain a type parameter inquiry"_port_en_US,
1049-
sf_.name()};
1066+
if (severity_) {
1067+
auto msg{
1068+
"Statement function '%s' should not contain a type parameter inquiry"_port_en_US};
1069+
msg.set_severity(*severity_);
1070+
return parser::Message{sf_.name(), std::move(msg), sf_.name()};
1071+
} else {
1072+
return std::nullopt;
1073+
}
10501074
}
10511075
Result operator()(const ProcedureDesignator &proc) const {
10521076
if (const Symbol * symbol{proc.GetSymbol()}) {
@@ -1064,16 +1088,23 @@ class StmtFunctionChecker
10641088
if (auto chars{
10651089
characteristics::Procedure::Characterize(proc, context_)}) {
10661090
if (!chars->CanBeCalledViaImplicitInterface()) {
1067-
return parser::Message(sf_.name(),
1068-
"Statement function '%s' should not reference function '%s' that requires an explicit interface"_port_en_US,
1069-
sf_.name(), symbol->name());
1091+
if (severity_) {
1092+
auto msg{
1093+
"Statement function '%s' should not reference function '%s' that requires an explicit interface"_port_en_US};
1094+
msg.set_severity(*severity_);
1095+
return parser::Message{
1096+
sf_.name(), std::move(msg), sf_.name(), symbol->name()};
1097+
}
10701098
}
10711099
}
10721100
}
10731101
if (proc.Rank() > 0) {
1074-
return parser::Message(sf_.name(),
1075-
"Statement function '%s' should not reference a function that returns an array"_port_en_US,
1076-
sf_.name());
1102+
if (severity_) {
1103+
auto msg{
1104+
"Statement function '%s' should not reference a function that returns an array"_port_en_US};
1105+
msg.set_severity(*severity_);
1106+
return parser::Message{sf_.name(), std::move(msg), sf_.name()};
1107+
}
10771108
}
10781109
return std::nullopt;
10791110
}
@@ -1083,9 +1114,12 @@ class StmtFunctionChecker
10831114
return result;
10841115
}
10851116
if (expr->Rank() > 0 && !UnwrapWholeSymbolOrComponentDataRef(*expr)) {
1086-
return parser::Message(sf_.name(),
1087-
"Statement function '%s' should not pass an array argument that is not a whole array"_port_en_US,
1088-
sf_.name());
1117+
if (severity_) {
1118+
auto msg{
1119+
"Statement function '%s' should not pass an array argument that is not a whole array"_port_en_US};
1120+
msg.set_severity(*severity_);
1121+
return parser::Message{sf_.name(), std::move(msg), sf_.name()};
1122+
}
10891123
}
10901124
}
10911125
return std::nullopt;
@@ -1094,6 +1128,7 @@ class StmtFunctionChecker
10941128
private:
10951129
const Symbol &sf_;
10961130
FoldingContext &context_;
1131+
std::optional<parser::Severity> severity_;
10971132
};
10981133

10991134
std::optional<parser::Message> CheckStatementFunction(

flang/lib/Evaluate/intrinsics.cpp

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2225,12 +2225,15 @@ std::optional<SpecificCall> IntrinsicInterface::Match(
22252225
if (const Symbol *whole{
22262226
UnwrapWholeSymbolOrComponentDataRef(actualForDummy[*dimArg])}) {
22272227
if (IsOptional(*whole) || IsAllocatableOrObjectPointer(whole)) {
2228-
if (rank == Rank::scalarIfDim || arrayRank.value_or(-1) == 1) {
2229-
messages.Say(
2230-
"The actual argument for DIM= is optional, pointer, or allocatable, and it is assumed to be present and equal to 1 at execution time"_port_en_US);
2231-
} else {
2232-
messages.Say(
2233-
"The actual argument for DIM= is optional, pointer, or allocatable, and may not be absent during execution; parenthesize to silence this warning"_warn_en_US);
2228+
if (context.languageFeatures().ShouldWarn(
2229+
common::UsageWarning::DimMustBePresent)) {
2230+
if (rank == Rank::scalarIfDim || arrayRank.value_or(-1) == 1) {
2231+
messages.Say(
2232+
"The actual argument for DIM= is optional, pointer, or allocatable, and it is assumed to be present and equal to 1 at execution time"_warn_en_US);
2233+
} else {
2234+
messages.Say(
2235+
"The actual argument for DIM= is optional, pointer, or allocatable, and may not be absent during execution; parenthesize to silence this warning"_warn_en_US);
2236+
}
22342237
}
22352238
}
22362239
}
@@ -3181,28 +3184,37 @@ std::optional<SpecificCall> IntrinsicProcTable::Implementation::Probe(
31813184

31823185
// If there was no exact match with a specific, try to match the related
31833186
// generic and convert the result to the specific required type.
3184-
for (auto specIter{specificRange.first}; specIter != specificRange.second;
3185-
++specIter) {
3186-
// We only need to check the cases with distinct generic names.
3187-
if (const char *genericName{specIter->second->generic}) {
3188-
if (specIter->second->useGenericAndForceResultType) {
3189-
auto genericRange{genericFuncs_.equal_range(genericName)};
3190-
for (auto genIter{genericRange.first}; genIter != genericRange.second;
3191-
++genIter) {
3192-
if (auto specificCall{
3193-
matchOrBufferMessages(*genIter->second, specificBuffer)}) {
3194-
// Force the call result type to the specific intrinsic result type
3195-
DynamicType newType{GetReturnType(*specIter->second, defaults_)};
3196-
context.messages().Say(
3197-
"argument types do not match specific intrinsic '%s' "
3198-
"requirements; using '%s' generic instead and converting the "
3199-
"result to %s if needed"_port_en_US,
3200-
call.name, genericName, newType.AsFortran());
3201-
specificCall->specificIntrinsic.name = call.name;
3202-
specificCall->specificIntrinsic.characteristics.value()
3203-
.functionResult.value()
3204-
.SetType(newType);
3205-
return specificCall;
3187+
if (context.languageFeatures().IsEnabled(common::LanguageFeature::
3188+
UseGenericIntrinsicWhenSpecificDoesntMatch)) {
3189+
for (auto specIter{specificRange.first}; specIter != specificRange.second;
3190+
++specIter) {
3191+
// We only need to check the cases with distinct generic names.
3192+
if (const char *genericName{specIter->second->generic}) {
3193+
if (specIter->second->useGenericAndForceResultType) {
3194+
auto genericRange{genericFuncs_.equal_range(genericName)};
3195+
for (auto genIter{genericRange.first}; genIter != genericRange.second;
3196+
++genIter) {
3197+
if (auto specificCall{
3198+
matchOrBufferMessages(*genIter->second, specificBuffer)}) {
3199+
// Force the call result type to the specific intrinsic result
3200+
// type
3201+
DynamicType newType{GetReturnType(*specIter->second, defaults_)};
3202+
if (context.languageFeatures().ShouldWarn(
3203+
common::LanguageFeature::
3204+
UseGenericIntrinsicWhenSpecificDoesntMatch)) {
3205+
context.messages().Say(
3206+
"Argument types do not match specific intrinsic '%s' "
3207+
"requirements; using '%s' generic instead and converting "
3208+
"the "
3209+
"result to %s if needed"_port_en_US,
3210+
call.name, genericName, newType.AsFortran());
3211+
}
3212+
specificCall->specificIntrinsic.name = call.name;
3213+
specificCall->specificIntrinsic.characteristics.value()
3214+
.functionResult.value()
3215+
.SetType(newType);
3216+
return specificCall;
3217+
}
32063218
}
32073219
}
32083220
}

flang/lib/Frontend/FrontendActions.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,8 @@ bool CodeGenAction::beginSourceFileAction() {
278278
ci.getInvocation().getSemanticsContext().targetCharacteristics(),
279279
ci.getParsing().allCooked(), ci.getInvocation().getTargetOpts().triple,
280280
kindMap, ci.getInvocation().getLoweringOpts(),
281-
ci.getInvocation().getFrontendOpts().envDefaults);
281+
ci.getInvocation().getFrontendOpts().envDefaults,
282+
ci.getInvocation().getFrontendOpts().features);
282283

283284
// Fetch module from lb, so we can set
284285
mlirModule = std::make_unique<mlir::ModuleOp>(lb.getModule());

flang/lib/Lower/Bridge.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4914,7 +4914,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
49144914

49154915
Fortran::evaluate::FoldingContext
49164916
Fortran::lower::LoweringBridge::createFoldingContext() const {
4917-
return {getDefaultKinds(), getIntrinsicTable(), getTargetCharacteristics()};
4917+
return {getDefaultKinds(), getIntrinsicTable(), getTargetCharacteristics(),
4918+
getLanguageFeatures()};
49184919
}
49194920

49204921
void Fortran::lower::LoweringBridge::lower(
@@ -4944,11 +4945,13 @@ Fortran::lower::LoweringBridge::LoweringBridge(
49444945
const Fortran::parser::AllCookedSources &cooked, llvm::StringRef triple,
49454946
fir::KindMapping &kindMap,
49464947
const Fortran::lower::LoweringOptions &loweringOptions,
4947-
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults)
4948+
const std::vector<Fortran::lower::EnvironmentDefault> &envDefaults,
4949+
const Fortran::common::LanguageFeatureControl &languageFeatures)
49484950
: semanticsContext{semanticsContext}, defaultKinds{defaultKinds},
49494951
intrinsics{intrinsics}, targetCharacteristics{targetCharacteristics},
49504952
cooked{&cooked}, context{context}, kindMap{kindMap},
4951-
loweringOptions{loweringOptions}, envDefaults{envDefaults} {
4953+
loweringOptions{loweringOptions}, envDefaults{envDefaults},
4954+
languageFeatures{languageFeatures} {
49524955
// Register the diagnostic handler.
49534956
context.getDiagEngine().registerHandler([](mlir::Diagnostic &diag) {
49544957
llvm::raw_ostream &os = llvm::errs();

flang/lib/Parser/expr-parsers.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,8 @@ constexpr auto primary{instrumented("primary"_en_US,
7777
construct<Expr>(Parser<StructureConstructor>{}),
7878
construct<Expr>(Parser<ArrayConstructor>{}),
7979
// PGI/XLF extension: COMPLEX constructor (x,y)
80-
extension<LanguageFeature::ComplexConstructor>(
81-
"nonstandard usage: generalized COMPLEX constructor"_port_en_US,
82-
construct<Expr>(parenthesized(
83-
construct<Expr::ComplexConstructor>(expr, "," >> expr)))),
80+
construct<Expr>(parenthesized(
81+
construct<Expr::ComplexConstructor>(expr, "," >> expr))),
8482
extension<LanguageFeature::PercentLOC>(
8583
"nonstandard usage: %LOC"_port_en_US,
8684
construct<Expr>("%LOC" >> parenthesized(construct<Expr::PercentLoc>(

0 commit comments

Comments
 (0)