Skip to content

Commit fceb9ce

Browse files
authored
[clang] consistently quote expressions in diagnostics (llvm#134769)
1 parent a388395 commit fceb9ce

File tree

53 files changed

+235
-199
lines changed

Some content is hidden

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

53 files changed

+235
-199
lines changed

clang/docs/InternalsManual.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,17 @@ Description:
416416
and the modifier indexes in the substitution are re-numbered accordingly. The
417417
substituted text must itself be a valid format string before substitution.
418418

419+
**"quoted" format**
420+
421+
Example:
422+
``"expression %quoted0 evaluates to 0"``
423+
Class:
424+
``String``
425+
Description:
426+
This is a simple formatter which adds quotes around the given string.
427+
This is useful when the argument could be a string in some cases, but
428+
another class in other cases, and it needs to be quoted consistently.
429+
419430
.. _internals-producing-diag:
420431

421432
Producing the Diagnostic

clang/docs/ReleaseNotes.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ Improvements to Clang's diagnostics
314314
- Clang now respects the current language mode when printing expressions in
315315
diagnostics. This fixes a bunch of `bool` being printed as `_Bool`, and also
316316
a bunch of HLSL types being printed as their C++ equivalents.
317+
- Clang now consistently quotes expressions in diagnostics.
317318
- When printing types for diagnostics, clang now doesn't suppress the scopes of
318319
template arguments contained within nested names.
319320
- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)

clang/include/clang/Basic/DiagnosticASTKinds.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ def note_constexpr_null_subobject : Note<
143143
"access array element of|perform pointer arithmetic on|"
144144
"access real component of|"
145145
"access imaginary component of}0 null pointer">;
146-
def note_constexpr_null_callee : Note<
147-
"'%0' evaluates to a null function pointer">;
146+
def note_constexpr_null_callee
147+
: Note<"%0 evaluates to a null function pointer">;
148148
def note_constexpr_function_param_value_unknown : Note<
149149
"function parameter %0 with unknown value cannot be used in a constant "
150150
"expression">;

clang/include/clang/Basic/DiagnosticParseKinds.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1639,7 +1639,7 @@ def note_omp_ctx_compatible_set_and_selector_for_property
16391639
"'match(%2={%1(%0)})'">;
16401640
def warn_omp_ctx_incompatible_score_for_property
16411641
: Warning<"the context selector '%0' in the context set '%1' cannot have a "
1642-
"score ('%2'); score ignored">,
1642+
"score (%quoted2); score ignored">,
16431643
InGroup<OpenMPClauses>;
16441644
def warn_omp_more_one_device_type_clause
16451645
: Warning<"more than one 'device_type' clause is specified">,

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -167,9 +167,10 @@ def err_expr_not_string_literal : Error<"expression is not a string literal">;
167167
def ext_predef_outside_function : Warning<
168168
"predefined identifier is only valid inside function">,
169169
InGroup<DiagGroup<"predefined-identifier-outside-function">>;
170-
def ext_init_from_predefined : ExtWarn<
171-
"initializing an array from a '%0' predefined identifier is a Microsoft extension">,
172-
InGroup<MicrosoftInitFromPredefined>;
170+
def ext_init_from_predefined
171+
: ExtWarn<"initializing an array from a %0 predefined identifier is a "
172+
"Microsoft extension">,
173+
InGroup<MicrosoftInitFromPredefined>;
173174
def ext_string_literal_from_predefined : ExtWarn<
174175
"expansion of predefined identifier '%0' to a string literal is a Microsoft extension">,
175176
InGroup<MicrosoftStringLiteralFromPredefined>;
@@ -3021,14 +3022,14 @@ def note_substituted_constraint_expr_is_ill_formed : Note<
30213022
"because substituted constraint expression is ill-formed%0">;
30223023
def note_constraint_references_error
30233024
: Note<"constraint depends on a previously diagnosed expression">;
3024-
def note_atomic_constraint_evaluated_to_false : Note<
3025-
"%select{and|because}0 '%1' evaluated to false">;
3026-
def note_concept_specialization_constraint_evaluated_to_false : Note<
3027-
"%select{and|because}0 '%1' evaluated to false">;
3025+
def note_atomic_constraint_evaluated_to_false
3026+
: Note<"%select{and|because}0 %1 evaluated to false">;
3027+
def note_concept_specialization_constraint_evaluated_to_false
3028+
: Note<"%select{and|because}0 %1 evaluated to false">;
30283029
def note_single_arg_concept_specialization_constraint_evaluated_to_false : Note<
30293030
"%select{and|because}0 %1 does not satisfy %2">;
3030-
def note_atomic_constraint_evaluated_to_false_elaborated : Note<
3031-
"%select{and|because}0 '%1' (%2 %3 %4) evaluated to false">;
3031+
def note_atomic_constraint_evaluated_to_false_elaborated
3032+
: Note<"%select{and|because}0 %1 (%2 %3 %4) evaluated to false">;
30323033
def note_is_deducible_constraint_evaluated_to_false : Note<
30333034
"cannot deduce template arguments for %0 from %1">;
30343035
def err_constrained_virtual_method : Error<
@@ -3048,14 +3049,14 @@ def note_expr_requirement_expr_substitution_error : Note<
30483049
"%select{and|because}0 '%1' would be invalid: %2">;
30493050
def note_expr_requirement_expr_unknown_substitution_error : Note<
30503051
"%select{and|because}0 '%1' would be invalid">;
3051-
def note_expr_requirement_noexcept_not_met : Note<
3052-
"%select{and|because}0 '%1' may throw an exception">;
3052+
def note_expr_requirement_noexcept_not_met
3053+
: Note<"%select{and|because}0 %1 may throw an exception">;
30533054
def note_expr_requirement_type_requirement_substitution_error : Note<
30543055
"%select{and|because}0 '%1' would be invalid: %2">;
30553056
def note_expr_requirement_type_requirement_unknown_substitution_error : Note<
30563057
"%select{and|because}0 '%1' would be invalid">;
3057-
def note_expr_requirement_constraints_not_satisfied : Note<
3058-
"%select{and|because}0 type constraint '%1' was not satisfied:">;
3058+
def note_expr_requirement_constraints_not_satisfied
3059+
: Note<"%select{and|because}0 type constraint %1 was not satisfied:">;
30593060
def note_expr_requirement_constraints_not_satisfied_simple : Note<
30603061
"%select{and|because}0 %1 does not satisfy %2:">;
30613062
def note_type_requirement_substitution_error : Note<
@@ -4356,9 +4357,9 @@ def note_reference_is_return_value : Note<"%0 returns a reference">;
43564357

43574358
def note_pointer_declared_here : Note<
43584359
"pointer %0 declared here">;
4359-
def warn_division_sizeof_ptr : Warning<
4360-
"'%0' will return the size of the pointer, not the array itself">,
4361-
InGroup<DiagGroup<"sizeof-pointer-div">>;
4360+
def warn_division_sizeof_ptr
4361+
: Warning<"%0 will return the size of the pointer, not the array itself">,
4362+
InGroup<DiagGroup<"sizeof-pointer-div">>;
43624363
def warn_division_sizeof_array : Warning<
43634364
"expression does not compute the number of elements in this array; element "
43644365
"type is %0, not %1">,
@@ -5581,8 +5582,9 @@ def note_function_member_spec_matched : Note<
55815582
def err_template_recursion_depth_exceeded : Error<
55825583
"recursive template instantiation exceeded maximum depth of %0">,
55835584
DefaultFatal, NoSFINAE;
5584-
def err_constraint_depends_on_self : Error<
5585-
"satisfaction of constraint '%0' depends on itself">, NoSFINAE;
5585+
def err_constraint_depends_on_self
5586+
: Error<"satisfaction of constraint %0 depends on itself">,
5587+
NoSFINAE;
55865588
def note_template_recursion_depth : Note<
55875589
"use -ftemplate-depth=N to increase recursive template instantiation depth">;
55885590

@@ -7098,8 +7100,8 @@ def warn_precedence_bitwise_rel : Warning<
70987100
InGroup<Parentheses>;
70997101
def note_precedence_bitwise_first : Note<
71007102
"place parentheses around the %0 expression to evaluate it first">;
7101-
def note_precedence_silence : Note<
7102-
"place parentheses around the '%0' expression to silence this warning">;
7103+
def note_precedence_silence : Note<"place parentheses around the %quoted0 "
7104+
"expression to silence this warning">;
71037105

71047106
def warn_precedence_conditional : Warning<
71057107
"operator '?:' has lower precedence than '%0'; '%0' will be evaluated first">,
@@ -7117,9 +7119,11 @@ def warn_consecutive_comparison : Warning<
71177119
def warn_enum_constant_in_bool_context : Warning<
71187120
"converting the enum constant to a boolean">,
71197121
InGroup<IntInBoolContext>, DefaultIgnore;
7120-
def warn_left_shift_in_bool_context : Warning<
7121-
"converting the result of '<<' to a boolean; did you mean '(%0) != 0'?">,
7122-
InGroup<IntInBoolContext>, DefaultIgnore;
7122+
def warn_left_shift_in_bool_context
7123+
: Warning<"converting the result of '<<' to a boolean; did you mean to "
7124+
"compare with '0'?">,
7125+
InGroup<IntInBoolContext>,
7126+
DefaultIgnore;
71237127
def warn_logical_instead_of_bitwise : Warning<
71247128
"use of logical '%0' with constant operand">,
71257129
InGroup<DiagGroup<"constant-logical-operand">>;
@@ -10331,9 +10335,11 @@ def warn_dangling_pointer_assignment : Warning<
1033110335
"object backing %select{|the pointer }0%1 "
1033210336
"will be destroyed at the end of the full-expression">,
1033310337
InGroup<DanglingAssignment>;
10334-
def warn_dangling_reference_captured : Warning<
10335-
"object whose reference is captured by '%0' will be destroyed at the end of "
10336-
"the full-expression">, InGroup<DanglingCapture>;
10338+
def warn_dangling_reference_captured
10339+
: Warning<"object whose reference is captured by %0 will be destroyed at "
10340+
"the end of "
10341+
"the full-expression">,
10342+
InGroup<DanglingCapture>;
1033710343
def warn_dangling_reference_captured_by_unknown : Warning<
1033810344
"object whose reference is captured will be destroyed at the end of "
1033910345
"the full-expression">, InGroup<DanglingCapture>;
@@ -13038,15 +13044,15 @@ def note_acc_atomic_too_many_stmts
1303813044
def note_acc_atomic_expected_binop : Note<"expected binary operation on right "
1303913045
"hand side of assignment operator">;
1304013046
def note_acc_atomic_mismatch_operand
13041-
: Note<"left hand side of assignment operation('%0') must match one side "
13042-
"of the sub-operation on the right hand side('%1' and '%2')">;
13047+
: Note<"left hand side of assignment operation(%0) must match one side "
13048+
"of the sub-operation on the right hand side(%1 and %2)">;
1304313049
def note_acc_atomic_mismatch_compound_operand
1304413050
: Note<"variable %select{|in unary expression|on right hand side of "
1304513051
"assignment|on left hand side of assignment|on left hand side of "
13046-
"compound assignment|on left hand side of assignment}2('%3') must "
13052+
"compound assignment|on left hand side of assignment}2(%3) must "
1304713053
"match variable used %select{|in unary expression|on right hand "
1304813054
"side of assignment|<not possible>|on left hand side of compound "
13049-
"assignment|on left hand side of assignment}0('%1') from the first "
13055+
"assignment|on left hand side of assignment}0(%1) from the first "
1305013056
"statement">;
1305113057
def err_acc_declare_required_clauses
1305213058
: Error<"no valid clauses specified in OpenACC 'declare' directive">;
@@ -13066,9 +13072,9 @@ def err_acc_multiple_references
1306613072
: Error<"variable referenced in '%0' clause of OpenACC 'declare' directive "
1306713073
"was already referenced">;
1306813074
def err_acc_routine_not_func
13069-
: Error<"OpenACC routine name '%0' does not name a function">;
13075+
: Error<"OpenACC routine name %0 does not name a function">;
1307013076
def err_acc_routine_overload_set
13071-
: Error<"OpenACC routine name '%0' names a set of overloads">;
13077+
: Error<"OpenACC routine name %0 names a set of overloads">;
1307213078
def err_acc_magic_static_in_routine
1307313079
: Error<"function static variables are not permitted in functions to which "
1307413080
"an OpenACC 'routine' directive applies">;

clang/lib/AST/ASTDiagnostic.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -512,8 +512,6 @@ void clang::FormatASTNodeDiagnosticArgument(
512512
const Expr *E = reinterpret_cast<Expr *>(Val);
513513
assert(E && "Received null Expr!");
514514
E->printPretty(OS, /*Helper=*/nullptr, Context.getPrintingPolicy());
515-
// FIXME: Include quotes when printing expressions.
516-
NeedQuotes = false;
517515
break;
518516
}
519517
}

clang/lib/Basic/Diagnostic.cpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,20 +1144,25 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
11441144

11451145
switch (Kind) {
11461146
// ---- STRINGS ----
1147-
case DiagnosticsEngine::ak_std_string: {
1148-
const std::string &S = getArgStdStr(ArgNo);
1149-
assert(ModifierLen == 0 && "No modifiers for strings yet");
1150-
EscapeStringForDiagnostic(S, OutStr);
1151-
break;
1152-
}
1147+
case DiagnosticsEngine::ak_std_string:
11531148
case DiagnosticsEngine::ak_c_string: {
1154-
const char *S = getArgCStr(ArgNo);
1155-
assert(ModifierLen == 0 && "No modifiers for strings yet");
1156-
1157-
// Don't crash if get passed a null pointer by accident.
1158-
if (!S)
1159-
S = "(null)";
1149+
StringRef S = [&]() -> StringRef {
1150+
if (Kind == DiagnosticsEngine::ak_std_string)
1151+
return getArgStdStr(ArgNo);
1152+
const char *SZ = getArgCStr(ArgNo);
1153+
// Don't crash if get passed a null pointer by accident.
1154+
return SZ ? SZ : "(null)";
1155+
}();
1156+
bool Quoted = false;
1157+
if (ModifierIs(Modifier, ModifierLen, "quoted")) {
1158+
Quoted = true;
1159+
OutStr.push_back('\'');
1160+
} else {
1161+
assert(ModifierLen == 0 && "unknown modifier for string");
1162+
}
11601163
EscapeStringForDiagnostic(S, OutStr);
1164+
if (Quoted)
1165+
OutStr.push_back('\'');
11611166
break;
11621167
}
11631168
// ---- INTEGERS ----

clang/lib/Sema/SemaChecking.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11685,7 +11685,10 @@ static void DiagnoseIntInBoolContext(Sema &S, Expr *E) {
1168511685
S.Diag(ExprLoc, diag::warn_left_shift_always)
1168611686
<< (Result.Val.getInt() != 0);
1168711687
else if (E->getType()->isSignedIntegerType())
11688-
S.Diag(ExprLoc, diag::warn_left_shift_in_bool_context) << E;
11688+
S.Diag(ExprLoc, diag::warn_left_shift_in_bool_context)
11689+
<< FixItHint::CreateInsertion(E->getBeginLoc(), "(")
11690+
<< FixItHint::CreateInsertion(S.getLocForEndOfToken(E->getEndLoc()),
11691+
") != 0");
1168911692
}
1169011693
}
1169111694

clang/test/AST/ByteCode/literals.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,7 @@ namespace CompoundLiterals {
920920
// null pointer suggests we're doing something odd during constant expression
921921
// evaluation: I think it's still taking 'x' as being null from the call to
922922
// f3() rather than tracking the assignment happening in the VLA.
923-
constexpr int f3(int *x, int (*y)[*(x=(int[]){1,2,3})]) { // both-warning {{object backing the pointer x will be destroyed at the end of the full-expression}}
923+
constexpr int f3(int *x, int (*y)[*(x=(int[]){1,2,3})]) { // both-warning {{object backing the pointer 'x' will be destroyed at the end of the full-expression}}
924924
return x[0]; // both-note {{read of dereferenced null pointer is not allowed in a constant expression}}
925925
}
926926
constexpr int h = f3(0,0); // both-error {{constexpr variable 'h' must be initialized by a constant expression}} \

clang/test/C/C11/n1285.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,51 +32,51 @@ void sink(int *);
3232

3333
int func_return(void) {
3434
int *p = f().a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
35-
p = f().a; // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
36-
p = g().a; // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
35+
p = f().a; // expected-warning {{object backing the pointer 'p' will be destroyed at the end of the full-expression}}
36+
p = g().a; // expected-warning {{object backing the pointer 'p' will be destroyed at the end of the full-expression}}
3737
sink(f().a); // Ok
3838
return *f().a; // Ok
3939
}
4040

4141
int ternary(void) {
4242
int *p = (1 ? (struct X){ 0 } : f()).a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
4343
int *r = (1 ? (union U){ 0 } : g()).a; // expected-warning {{temporary whose address is used as value of local variable 'r' will be destroyed at the end of the full-expression}}
44-
p = (1 ? (struct X){ 0 } : f()).a; // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
44+
p = (1 ? (struct X){ 0 } : f()).a; // expected-warning {{object backing the pointer 'p' will be destroyed at the end of the full-expression}}
4545
sink((1 ? (struct X){ 0 } : f()).a); // Ok
4646

4747
// This intentionally gets one diagnostic in C and two in C++. In C, the
4848
// compound literal results in an lvalue, not an rvalue as it does in C++. So
4949
// only one branch results in a temporary in C but both branches do in C++.
5050
int *q = 1 ? (struct X){ 0 }.a : f().a; // expected-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}} \
5151
cpp-warning {{temporary whose address is used as value of local variable 'q' will be destroyed at the end of the full-expression}}
52-
q = 1 ? (struct X){ 0 }.a : f().a; // expected-warning {{object backing the pointer q will be destroyed at the end of the full-expression}} \
53-
cpp-warning {{object backing the pointer q will be destroyed at the end of the full-expression}}
54-
q = 1 ? (struct X){ 0 }.a : g().a; // expected-warning {{object backing the pointer q will be destroyed at the end of the full-expression}} \
55-
cpp-warning {{object backing the pointer q will be destroyed at the end of the full-expression}}
52+
q = 1 ? (struct X){ 0 }.a : f().a; // expected-warning {{object backing the pointer 'q' will be destroyed at the end of the full-expression}} \
53+
cpp-warning {{object backing the pointer 'q' will be destroyed at the end of the full-expression}}
54+
q = 1 ? (struct X){ 0 }.a : g().a; // expected-warning {{object backing the pointer 'q' will be destroyed at the end of the full-expression}} \
55+
cpp-warning {{object backing the pointer 'q' will be destroyed at the end of the full-expression}}
5656
sink(1 ? (struct X){ 0 }.a : f().a); // Ok
5757
return *(1 ? (struct X){ 0 }.a : f().a); // Ok
5858
}
5959

6060
int comma(void) {
6161
struct X x;
6262
int *p = ((void)0, x).a; // c-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
63-
p = ((void)0, x).a; // c-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
63+
p = ((void)0, x).a; // c-warning {{object backing the pointer 'p' will be destroyed at the end of the full-expression}}
6464
sink(((void)0, x).a); // Ok
6565
return *(((void)0, x).a);// Ok
6666
}
6767

6868
int cast(void) {
6969
struct X x;
7070
int *p = ((struct X)x).a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
71-
p = ((struct X)x).a; // expected-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
71+
p = ((struct X)x).a; // expected-warning {{object backing the pointer 'p' will be destroyed at the end of the full-expression}}
7272
sink(((struct X)x).a); // Ok
7373
return *(((struct X)x).a); // Ok
7474
}
7575

7676
int assign(void) {
7777
struct X x, s;
7878
int *p = (x = s).a; // c-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}}
79-
p = (x = s).a; // c-warning {{object backing the pointer p will be destroyed at the end of the full-expression}}
79+
p = (x = s).a; // c-warning {{object backing the pointer 'p' will be destroyed at the end of the full-expression}}
8080
sink((x = s).a); // Ok
8181
return *((x = s).a); // Ok
8282
}

0 commit comments

Comments
 (0)