Skip to content

Commit 976f149

Browse files
committed
[NFC] Introduce DiagRef and use it throughout the parser
The goal is to have a lightweight way to pass an unapplied diagnostic to general routines. Constructing a Diagnostic is quite expensive as something we're potentially doing in hot paths, as opposed to just when we're actually emitting the diagnostic. This design allows the expense to be delayed until we need it. I've also optimized the Diagnostic constructor to avoid copying arguments unnecessarily; this is a relatively small expense, since arguments are POD, but there's really no good reason not to do it.
1 parent 78df826 commit 976f149

File tree

7 files changed

+166
-52
lines changed

7 files changed

+166
-52
lines changed

include/swift/AST/DiagnosticEngine.h

Lines changed: 121 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ namespace swift {
8888
};
8989
}
9090

91+
template <class... ArgTypes>
92+
using DiagArgTuple =
93+
std::tuple<typename detail::PassArgument<ArgTypes>::type...>;
94+
9195
/// A family of wrapper types for compiler data types that forces its
9296
/// underlying data to be formatted with full qualification.
9397
///
@@ -476,20 +480,28 @@ namespace swift {
476480
friend DiagnosticEngine;
477481
friend class InFlightDiagnostic;
478482

483+
Diagnostic(DiagID ID) : ID(ID) {}
484+
479485
public:
480486
// All constructors are intentionally implicit.
481487
template<typename ...ArgTypes>
482488
Diagnostic(Diag<ArgTypes...> ID,
483489
typename detail::PassArgument<ArgTypes>::type... VArgs)
484-
: ID(ID.ID) {
485-
DiagnosticArgument DiagArgs[] = {
486-
DiagnosticArgument(0), std::move(VArgs)...
487-
};
488-
Args.append(DiagArgs + 1, DiagArgs + 1 + sizeof...(VArgs));
490+
: Diagnostic(ID.ID) {
491+
Args.reserve(sizeof...(ArgTypes));
492+
gatherArgs(VArgs...);
489493
}
490494

491495
/*implicit*/Diagnostic(DiagID ID, ArrayRef<DiagnosticArgument> Args)
492496
: ID(ID), Args(Args.begin(), Args.end()) {}
497+
498+
template <class... ArgTypes>
499+
static Diagnostic fromTuple(Diag<ArgTypes...> id,
500+
const DiagArgTuple<ArgTypes...> &tuple) {
501+
Diagnostic result(id.ID);
502+
result.gatherArgsFromTuple<DiagArgTuple<ArgTypes...>, 0, ArgTypes...>(tuple);
503+
return result;
504+
}
493505

494506
// Accessors.
495507
DiagID getID() const { return ID; }
@@ -528,6 +540,37 @@ namespace swift {
528540

529541
void addChildNote(Diagnostic &&D);
530542
void insertChildNote(unsigned beforeIndex, Diagnostic &&D);
543+
544+
private:
545+
// gatherArgs could just be `Args.emplace_back(args)...;` if C++
546+
// allowed pack expansions in statement context.
547+
548+
// Base case.
549+
void gatherArgs() {}
550+
551+
// Pull one off the pack.
552+
template <class ArgType, class... RemainingArgTypes>
553+
void gatherArgs(ArgType arg, RemainingArgTypes... remainingArgs) {
554+
Args.emplace_back(arg);
555+
gatherArgs(remainingArgs...);
556+
}
557+
558+
// gatherArgsFromTuple could just be
559+
// `Args.emplace_back(std::get<packIndexOf<ArgTypes>>(tuple))...;`
560+
// in a better world.
561+
562+
// Base case.
563+
template <class Tuple, size_t Index>
564+
void gatherArgsFromTuple(const Tuple &tuple) {}
565+
566+
// Pull one off the pack.
567+
template <class Tuple, size_t Index,
568+
class ArgType, class... RemainingArgTypes>
569+
void gatherArgsFromTuple(const Tuple &tuple) {
570+
Args.emplace_back(std::move(std::get<Index>(tuple)));
571+
gatherArgsFromTuple<Tuple, Index + 1, RemainingArgTypes...>(
572+
std::move(tuple));
573+
}
531574
};
532575

533576
/// A diagnostic that has no input arguments, so it is trivially-destructable.
@@ -866,6 +909,73 @@ namespace swift {
866909
DiagnosticState &operator=(DiagnosticState &&) = default;
867910
};
868911

912+
/// A lightweight reference to a diagnostic that's been fully applied to
913+
/// its arguments. This allows a general routine (in the parser, say) to
914+
/// be customized to emit an arbitrary diagnostic without needing to
915+
/// eagerly construct a full Diagnostic. Like ArrayRef and function_ref,
916+
/// this stores a reference to what's likely to be a temporary, so it
917+
/// should only be used as a function parameter. If you need to persist
918+
/// the diagnostic, you'll have to call createDiagnostic().
919+
///
920+
/// You can initialize a DiagRef parameter in one of two ways:
921+
/// - passing a Diag<> as the argument, e.g.
922+
/// diag::circular_reference
923+
/// or
924+
/// - constructing it with a Diag and its arguments, e.g.
925+
/// {diag::circular_protocol_def, {proto->getName()}}
926+
///
927+
/// It'd be nice to let people write `{diag::my_error, arg0, arg1}`
928+
/// instead of `{diag::my_error, {arg0, arg1}}`, but we can't: the
929+
/// temporary needs to be created in the calling context.
930+
class DiagRef {
931+
DiagID id;
932+
933+
/// If this is null, then id is a Diag<> and there are no arguments.
934+
Diagnostic (*createFn)(DiagID id, const void *opaqueStorage);
935+
const void *opaqueStorage;
936+
937+
public:
938+
/// Construct a diagnostic from a diagnostic ID that's known to not take
939+
/// arguments.
940+
DiagRef(Diag<> id)
941+
: id(id.ID), createFn(nullptr), opaqueStorage(nullptr) {}
942+
943+
/// Construct a diagnostic from a diagnostic ID and its arguments.
944+
template <class... ArgTypes>
945+
DiagRef(Diag<ArgTypes...> id, const DiagArgTuple<ArgTypes...> &tuple)
946+
: id(id.ID),
947+
createFn(&createFromTuple<ArgTypes...>),
948+
opaqueStorage(&tuple) {}
949+
950+
// A specialization of the general constructor above for diagnostics
951+
// with no arguments; this is a useful optimization when a DiagRef
952+
// is constructed generically.
953+
DiagRef(Diag<> id, const DiagArgTuple<> &tuple)
954+
: DiagRef(id) {}
955+
956+
/// Return the diagnostic ID that this will emit.
957+
DiagID getID() const {
958+
return id;
959+
}
960+
961+
/// Create a full Diagnostic. It's safe to do this multiple times on
962+
/// a single DiagRef.
963+
Diagnostic createDiagnostic() {
964+
if (!createFn) {
965+
return Diagnostic(Diag<> {id});
966+
} else {
967+
return createFn(id, opaqueStorage);
968+
}
969+
}
970+
971+
private:
972+
template <class... ArgTypes>
973+
static Diagnostic createFromTuple(DiagID id, const void *opaqueStorage) {
974+
auto tuple = static_cast<const DiagArgTuple<ArgTypes...> *>(opaqueStorage);
975+
return Diagnostic::fromTuple(Diag<ArgTypes...> {id}, *tuple);
976+
}
977+
};
978+
869979
/// Class responsible for formatting diagnostics and presenting them
870980
/// to the user.
871981
class DiagnosticEngine {
@@ -1113,6 +1223,12 @@ namespace swift {
11131223
return diagnose(Loc, Diagnostic(ID, std::move(Args)...));
11141224
}
11151225

1226+
/// Emit the given lazily-applied diagnostic at the specified
1227+
/// source location.
1228+
InFlightDiagnostic diagnose(SourceLoc loc, DiagRef diag) {
1229+
return diagnose(loc, diag.createDiagnostic());
1230+
}
1231+
11161232
/// Delete an API that may lead clients to avoid specifying source location.
11171233
template<typename ...ArgTypes>
11181234
InFlightDiagnostic

include/swift/Parse/Parser.h

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -729,28 +729,28 @@ class Parser {
729729
}
730730

731731
public:
732-
InFlightDiagnostic diagnose(SourceLoc Loc, Diagnostic Diag) {
732+
InFlightDiagnostic diagnose(SourceLoc Loc, DiagRef Diag) {
733733
if (Diags.isDiagnosticPointsToFirstBadToken(Diag.getID()) &&
734734
Loc == Tok.getLoc() && Tok.isAtStartOfLine())
735735
Loc = getEndOfPreviousLoc();
736736
return Diags.diagnose(Loc, Diag);
737737
}
738738

739-
InFlightDiagnostic diagnose(Token Tok, Diagnostic Diag) {
739+
InFlightDiagnostic diagnose(Token Tok, DiagRef Diag) {
740740
return diagnose(Tok.getLoc(), Diag);
741741
}
742742

743743
template<typename ...DiagArgTypes, typename ...ArgTypes>
744744
InFlightDiagnostic diagnose(SourceLoc Loc, Diag<DiagArgTypes...> DiagID,
745745
ArgTypes &&...Args) {
746-
return diagnose(Loc, Diagnostic(DiagID, std::forward<ArgTypes>(Args)...));
746+
return diagnose(Loc, {DiagID, {std::forward<ArgTypes>(Args)...}});
747747
}
748748

749749
template<typename ...DiagArgTypes, typename ...ArgTypes>
750750
InFlightDiagnostic diagnose(Token Tok, Diag<DiagArgTypes...> DiagID,
751751
ArgTypes &&...Args) {
752752
return diagnose(Tok.getLoc(),
753-
Diagnostic(DiagID, std::forward<ArgTypes>(Args)...));
753+
{DiagID, {std::forward<ArgTypes>(Args)...}});
754754
}
755755

756756
/// Add a fix-it to remove the space in consecutive identifiers.
@@ -809,68 +809,68 @@ class Parser {
809809
/// its name in \p Result. Otherwise, emit an error.
810810
///
811811
/// \returns false on success, true on error.
812-
bool parseIdentifier(Identifier &Result, SourceLoc &Loc, const Diagnostic &D,
812+
bool parseIdentifier(Identifier &Result, SourceLoc &Loc, DiagRef D,
813813
bool diagnoseDollarPrefix);
814814

815815
/// Consume an identifier with a specific expected name. This is useful for
816816
/// contextually sensitive keywords that must always be present.
817817
bool parseSpecificIdentifier(StringRef expected, SourceLoc &Loc,
818-
const Diagnostic &D);
818+
DiagRef D);
819819

820820
template<typename ...DiagArgTypes, typename ...ArgTypes>
821821
bool parseIdentifier(Identifier &Result, SourceLoc &L,
822822
bool diagnoseDollarPrefix, Diag<DiagArgTypes...> ID,
823823
ArgTypes... Args) {
824-
return parseIdentifier(Result, L, Diagnostic(ID, Args...),
824+
return parseIdentifier(Result, L, {ID, {Args...}},
825825
diagnoseDollarPrefix);
826826
}
827827

828828
template<typename ...DiagArgTypes, typename ...ArgTypes>
829829
bool parseSpecificIdentifier(StringRef expected,
830830
Diag<DiagArgTypes...> ID, ArgTypes... Args) {
831831
SourceLoc L;
832-
return parseSpecificIdentifier(expected, L, Diagnostic(ID, Args...));
832+
return parseSpecificIdentifier(expected, L, {ID, {Args...}});
833833
}
834834

835835
/// Consume an identifier or operator if present and return its name
836836
/// in \p Result. Otherwise, emit an error and return true.
837837
bool parseAnyIdentifier(Identifier &Result, SourceLoc &Loc,
838-
const Diagnostic &D, bool diagnoseDollarPrefix);
838+
DiagRef D, bool diagnoseDollarPrefix);
839839

840840
template<typename ...DiagArgTypes, typename ...ArgTypes>
841841
bool parseAnyIdentifier(Identifier &Result, bool diagnoseDollarPrefix,
842842
Diag<DiagArgTypes...> ID, ArgTypes... Args) {
843843
SourceLoc L;
844-
return parseAnyIdentifier(Result, L, Diagnostic(ID, Args...),
844+
return parseAnyIdentifier(Result, L, {ID, {Args...}},
845845
diagnoseDollarPrefix);
846846
}
847847

848848
/// \brief Parse an unsigned integer and returns it in \p Result. On failure
849849
/// emit the specified error diagnostic, and a note at the specified note
850850
/// location.
851851
bool parseUnsignedInteger(unsigned &Result, SourceLoc &Loc,
852-
const Diagnostic &D);
852+
DiagRef D);
853853

854854
/// The parser expects that \p K is next token in the input. If so,
855855
/// it is consumed and false is returned.
856856
///
857857
/// If the input is malformed, this emits the specified error diagnostic.
858-
bool parseToken(tok K, SourceLoc &TokLoc, const Diagnostic &D);
858+
bool parseToken(tok K, SourceLoc &TokLoc, DiagRef D);
859859

860860
template<typename ...DiagArgTypes, typename ...ArgTypes>
861861
bool parseToken(tok K, Diag<DiagArgTypes...> ID, ArgTypes... Args) {
862862
SourceLoc L;
863-
return parseToken(K, L, Diagnostic(ID, Args...));
863+
return parseToken(K, L, {ID, {Args...}});
864864
}
865865
template<typename ...DiagArgTypes, typename ...ArgTypes>
866866
bool parseToken(tok K, SourceLoc &L,
867867
Diag<DiagArgTypes...> ID, ArgTypes... Args) {
868-
return parseToken(K, L, Diagnostic(ID, Args...));
868+
return parseToken(K, L, {ID, {Args...}});
869869
}
870870

871871
/// Parse the specified expected token and return its location on success. On failure, emit the specified
872872
/// error diagnostic, a note at the specified note location, and return the location of the previous token.
873-
bool parseMatchingToken(tok K, SourceLoc &TokLoc, Diagnostic ErrorDiag,
873+
bool parseMatchingToken(tok K, SourceLoc &TokLoc, DiagRef ErrorDiag,
874874
SourceLoc OtherLoc);
875875

876876
/// Returns the proper location for a missing right brace, parenthesis, etc.
@@ -903,7 +903,7 @@ class Parser {
903903

904904
/// Parse a comma separated list of some elements.
905905
ParserStatus parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
906-
bool AllowSepAfterLast, Diag<> ErrorDiag,
906+
bool AllowSepAfterLast, DiagRef RightErrorDiag,
907907
llvm::function_ref<ParserStatus()> callback);
908908

909909
void consumeTopLevelDecl(ParserPosition BeginParserPosition,
@@ -1184,7 +1184,7 @@ class Parser {
11841184
/// Parse a version tuple of the form x[.y[.z]]. Returns true if there was
11851185
/// an error parsing.
11861186
bool parseVersionTuple(llvm::VersionTuple &Version, SourceRange &Range,
1187-
const Diagnostic &D);
1187+
DiagRef D);
11881188

11891189
bool isParameterSpecifier() {
11901190
if (Tok.is(tok::kw_inout)) return true;
@@ -1832,7 +1832,7 @@ class Parser {
18321832
/// unqualified-decl-name:
18331833
/// unqualified-decl-base-name
18341834
/// unqualified-decl-base-name '(' ((identifier | '_') ':') + ')'
1835-
DeclNameRef parseDeclNameRef(DeclNameLoc &loc, const Diagnostic &diag,
1835+
DeclNameRef parseDeclNameRef(DeclNameLoc &loc, DiagRef diag,
18361836
DeclNameOptions flags);
18371837

18381838
/// Parse macro expansion.
@@ -1843,7 +1843,7 @@ class Parser {
18431843
SourceLoc &poundLoc, DeclNameLoc &macroNameLoc, DeclNameRef &macroNameRef,
18441844
SourceLoc &leftAngleLoc, SmallVectorImpl<TypeRepr *> &genericArgs,
18451845
SourceLoc &rightAngleLoc, ArgumentList *&argList, bool isExprBasic,
1846-
const Diagnostic &diag);
1846+
DiagRef diag);
18471847

18481848
ParserResult<Expr> parseExprIdentifier(bool allowKeyword);
18491849
Expr *parseExprEditorPlaceholder(Token PlaceholderTok,

lib/Parse/ParseDecl.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ ParserResult<AvailableAttr> Parser::parseExtendedAvailabilitySpecList(
629629
bool VerArgWasEmpty = VerArg.empty();
630630
if (parseVersionTuple(
631631
VerArg.Version, VerArg.Range,
632-
Diagnostic(diag::attr_availability_expected_version, AttrName))) {
632+
{diag::attr_availability_expected_version, {AttrName}})) {
633633
AnyArgumentInvalid = true;
634634
if (peekToken().isAny(tok::r_paren, tok::comma))
635635
consumeToken();
@@ -2251,7 +2251,7 @@ ParserStatus Parser::parsePlatformVersionInList(StringRef AttrName,
22512251
llvm::VersionTuple VerTuple;
22522252
SourceRange VersionRange;
22532253
if (parseVersionTuple(VerTuple, VersionRange,
2254-
Diagnostic(diag::attr_availability_expected_version, AttrName))) {
2254+
{diag::attr_availability_expected_version, {AttrName}})) {
22552255
return makeParserError();
22562256
}
22572257

@@ -2745,7 +2745,7 @@ Parser::parseMacroRoleAttribute(
27452745
/// omitted; the identifier written by the user otherwise.
27462746
static std::optional<Identifier> parseSingleAttrOptionImpl(
27472747
Parser &P, SourceLoc Loc, SourceRange &AttrRange, StringRef AttrName,
2748-
DeclAttrKind DK, bool allowOmitted, Diagnostic nonIdentifierDiagnostic) {
2748+
DeclAttrKind DK, bool allowOmitted, DiagRef nonIdentifierDiagnostic) {
27492749
SWIFT_DEFER {
27502750
AttrRange = SourceRange(Loc, P.PreviousLoc);
27512751
};
@@ -2793,7 +2793,7 @@ parseSingleAttrOptionIdentifier(Parser &P, SourceLoc Loc,
27932793
DeclAttrKind DK, bool allowOmitted = false) {
27942794
return parseSingleAttrOptionImpl(
27952795
P, Loc, AttrRange, AttrName, DK, allowOmitted,
2796-
{ diag::attr_expected_option_identifier, AttrName });
2796+
{diag::attr_expected_option_identifier, {AttrName}});
27972797
}
27982798

27992799
/// Parses a (possibly optional) argument for an attribute containing a single identifier from a known set of
@@ -2819,8 +2819,8 @@ parseSingleAttrOption(Parser &P, SourceLoc Loc, SourceRange &AttrRange,
28192819
auto parsedIdentifier = parseSingleAttrOptionImpl(
28202820
P, Loc, AttrRange,AttrName, DK,
28212821
/*allowOmitted=*/valueIfOmitted.has_value(),
2822-
Diagnostic(diag::attr_expected_option_such_as, AttrName,
2823-
options.front().first.str()));
2822+
{diag::attr_expected_option_such_as,
2823+
{AttrName, options.front().first.str()}});
28242824
if (!parsedIdentifier)
28252825
return std::nullopt;
28262826

@@ -4043,7 +4043,7 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
40434043

40444044
bool Parser::parseVersionTuple(llvm::VersionTuple &Version,
40454045
SourceRange &Range,
4046-
const Diagnostic &D) {
4046+
DiagRef D) {
40474047
// A version number is either an integer (8), a float (8.1), or a
40484048
// float followed by a dot and an integer (8.1.0).
40494049
if (!Tok.isAny(tok::integer_literal, tok::floating_literal)) {

lib/Parse/ParseExpr.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2275,7 +2275,7 @@ static bool tryParseArgLabelList(Parser &P, Parser::DeclNameOptions flags,
22752275
}
22762276

22772277
DeclNameRef Parser::parseDeclNameRef(DeclNameLoc &loc,
2278-
const Diagnostic &diag,
2278+
DiagRef diag,
22792279
DeclNameOptions flags) {
22802280
// Consume the base name.
22812281
DeclBaseName baseName;
@@ -2338,7 +2338,7 @@ ParserStatus Parser::parseFreestandingMacroExpansion(
23382338
SourceLoc &poundLoc, DeclNameLoc &macroNameLoc, DeclNameRef &macroNameRef,
23392339
SourceLoc &leftAngleLoc, SmallVectorImpl<TypeRepr *> &genericArgs,
23402340
SourceLoc &rightAngleLoc, ArgumentList *&argList, bool isExprBasic,
2341-
const Diagnostic &diag) {
2341+
DiagRef diag) {
23422342
SourceLoc poundEndLoc = Tok.getRange().getEnd();
23432343
poundLoc = consumeToken(tok::pound);
23442344

0 commit comments

Comments
 (0)