Skip to content
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.

Sign up for GitHub

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

Emit a diagnostic warning about parsing ambiguity in a conjunction #55

Closed
wants to merge 16 commits into from
Closed

Emit a diagnostic warning about parsing ambiguity in a conjunction #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
378e907
feat: first version of implementation
BaLiKfromUA May 27, 2024
53c7d38
test: add basic test for parsing
BaLiKfromUA May 28, 2024
98b0a59
minor refactoring
BaLiKfromUA May 28, 2024
32c3567
fix: add suppressing of introduced warning
BaLiKfromUA May 30, 2024
99e6528
feat: add warning for parsing of '&' token in reflection expr
BaLiKfromUA May 30, 2024
44b62f0
style: revert some changes
BaLiKfromUA May 30, 2024
a13d5e8
style: remove tabs
BaLiKfromUA May 30, 2024
6772510
style: remove tabs
BaLiKfromUA May 30, 2024
7be17b0
refactor: improve wording in warning
BaLiKfromUA May 31, 2024
bfdfdd6
refactor: replace hardcoded lines in tests
BaLiKfromUA Jun 9, 2024
a736536
fix: false positive warning during parsing rvalue of type `meta::info`
BaLiKfromUA Jun 9, 2024
3554cdb
refactor: rename and move test to Reflection folder
BaLiKfromUA Jun 9, 2024
af2a8c1
refactor: change wording in warning
BaLiKfromUA Jun 9, 2024
5bb406e
fix typo
BaLiKfromUA Jun 9, 2024
a2aead4
test: add more test cases
BaLiKfromUA Jun 9, 2024
07bba7b
test: add more test cases
BaLiKfromUA Jun 11, 2024
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
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticGroups.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1532,3 +1532,6 @@ def BitIntExtension : DiagGroup<"bit-int-extension">;

// Warnings about misuse of ExtractAPI options.
def ExtractAPIMisuse : DiagGroup<"extractapi-misuse">;

// Warning related to reflection
def ReflExprParsingAmbiguity : DiagGroup<"reflect-parsing-ambiguity">;
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1713,6 +1713,12 @@ let CategoryName = "Reflection Issue" in {
def err_cannot_reflect_entity : Error<"cannot reflect entity">;
def err_expansion_statements_disabled : Error<
"'template for' statements are not enabled; use '-fexpansion-statements'">;

def warn_parsing_ambiguity_in_refl_expression_with_reference_types : Warning <
"token '%1' binds left to type '%0 %1'; "
"did you mean '(^%0) %1'?"
>,
InGroup<ReflExprParsingAmbiguity>;
}

let CategoryName = "Generics Issue" in {
Expand Down
15 changes: 15 additions & 0 deletions clang/lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6750,6 +6750,21 @@ void Parser::ParseDeclaratorInternal(Declarator &D,
// Is a reference
DeclSpec DS(AttrFactory);

// Complain about parsing ambiguity of parsing reference types
// inside expressions with reflection in C++2x
if (D.getContext() == DeclaratorContext::ReflectOperator &&
!Tok.isOneOf(tok::r_paren, tok::semi)) {
// parser already consumed '^' token before setting this context
assert((Kind == tok::ampamp || Kind == tok::amp) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, there are a lot more cases here. I'm not sure this approach will work (although it might - not really sure one way or the other).

r == ^int& ? 1 : 0
condition ? ^int& : ^int;
^int&(*)(bool)
^int& != r
^int& == r
constexpr auto r {^int&};

In addition to those, in the presence of some user-defined

consteval result_type operator???(std::meta::info R, MyStupidClass);

You could have ^int& + o, along with: -, *, /, %, ], |, || <=, >=, >.

Those are all the cases I can think of off the top of my head, but I may have forgotten some.

Copy link
Author

Choose a reason for hiding this comment

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

thank you! gonna think more how to address it

"expected '&' or '&&'");
Diag(Loc,
diag::warn_parsing_ambiguity_in_refl_expression_with_reference_types)
<< (D.hasName()
? Actions.GetNameForDeclarator(D).getName().getAsString()
: "T")
<< tok::getPunctuatorSpelling(Kind);
}

// Complain about rvalue references in C++03, but then go on and build
// the declarator.
if (Kind == tok::ampamp)
Expand Down
67 changes: 67 additions & 0 deletions clang/test/Reflection/ambig-reflect-expr-parsing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//===----------------------------------------------------------------------===//
//
// Copyright 2024 Bloomberg Finance L.P.
//
// 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
//
//===----------------------------------------------------------------------===//
//
// RUN: %clang_cc1 -std=c++26 -freflection -Wno-parentheses -verify %s


enum MyEnum { x, y, e = -1, f, z = 99 };

void func(MyEnum && x) { // ok
MyEnum value{};
MyEnum& ref = value;

constexpr auto reflValue = ^value;

constexpr bool test_comparison_0 = reflValue != (^MyEnum) && true; // ok
constexpr bool test_comparison_1 = (reflValue != (^MyEnum) && true); // ok

constexpr bool test_comparison_2 = ^MyEnum != (^MyEnum) && true; // ok
constexpr bool test_comparison_3 = (^MyEnum != (^MyEnum) && true); // ok

constexpr bool test_comparison_4 = (reflValue != (^MyEnum) & true); // ok

constexpr bool test_comparison_5 = reflValue != ^MyEnum && true;
// expected-warning@-1 {{token '&&' binds left to type 'T &&'; did you mean '(^T) &&'?}}
// expected-error@-2 {{expected ';' at end of declaration}}
constexpr bool test_comparison_6 = reflValue != ^MyEnum & true;
// expected-warning@-1 {{token '&' binds left to type 'T &'; did you mean '(^T) &'?}}
// expected-error@-2 {{expected ';' at end of declaration}}

constexpr bool test_comparison_7 = ^MyEnum& == reflValue; // ok
constexpr bool test_comparison_8 = ^MyEnum& != reflValue; // ok

constexpr bool test_comparison_9 = ^MyEnum& < reflValue;
// expected-error@-1 {{invalid operands to binary expression ('meta::info' and 'const meta::info')}}
constexpr bool test_comparison_10 = ^MyEnum& > reflValue;
// expected-error@-1 {{invalid operands to binary expression ('meta::info' and 'const meta::info')}}
constexpr bool test_comparison_11 = ^MyEnum& <= reflValue;
// expected-error@-1 {{invalid operands to binary expression ('meta::info' and 'const meta::info')}}
constexpr bool test_comparison_12 = ^MyEnum& >= reflValue;
// expected-error@-1 {{invalid operands to binary expression ('meta::info' and 'const meta::info')}}

constexpr auto test_comparison_13 = reflValue != ^MyEnum&& || true; // ok
constexpr auto test_comparison_14 = reflValue != ^MyEnum& | true; // ok

constexpr auto test_comparison_15 = reflValue != ^MyEnum&; // ok
constexpr auto test_comparison_16 = reflValue == ^MyEnum&; // ok

constexpr auto test_ref_type_0 = ^MyEnum &; // ok
constexpr auto test_ref_type_1 = ^MyEnum &&; // ok
constexpr auto test_ref_type_2 = (^MyEnum &); // ok
constexpr auto test_ref_type_3 = (^MyEnum &&); // ok

constexpr auto ternary_comparison_test_0 = reflValue == ^MyEnum& ? 1 : 0; // ok
constexpr auto ternary_comparison_test_1 = (2 > 0) ? ^MyEnum& : ^MyEnum; // ok
constexpr auto ternary_comparison_test_2 = (2 > 0) ? ^MyEnum : ^MyEnum&; // ok

constexpr auto test_init_0 {^MyEnum&}; // ok
constexpr auto test_type_id_0 = ^MyEnum&(*)(bool); // ok
}