Skip to content

[cling] Emit proper diagnostics in DynamicLookup for AutoType #18170

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

devajithvs
Copy link
Contributor

@devajithvs devajithvs commented Mar 27, 2025

Replace AST dumps with proper error messages in dynamic lookup when dealing with auto, which is unsupported for lookup

Changes this:

root [0] auto var = someexpr();
ROOT_prompt_0:1:1: error: Syntax error
auto var = someexpr();
^
FunctionDecl 0x60cb530887f0 <input_line_8:1:1, ROOT_prompt_0:3:1> input_line_8:1:6 __cling_Un1Qu30 'void (void *)'
|-ParmVarDecl 0x60cb53088738 <col:22, col:28> col:28 vpClingValue 'void *'
|-CompoundStmt 0x60cb53088c30 <col:42, ROOT_prompt_0:3:1>
| |-DeclStmt 0x60cb53088c10 <line:1:1, col:22>
| | `-VarDecl 0x60cb53088910 <col:1, col:21> col:6 var 'auto' cinit
| |   `-CallExpr 0x60cb53088bc0 <col:12, col:21> '<dependent type>'
| |     `-DeclRefExpr 0x60cb53088b80 <col:12> '<dependent type>' lvalue Var 0x60cb53088aa8 'someexpr' '<dependent type>'
| `-NullStmt 0x60cb53088c28 <line:2:1>
|-AnnotateAttr 0x60cb53088a00 <<invalid sloc>> Implicit R"ATTRDUMP(__ResolveAtRuntime)ATTRDUMP"
`-AnnotateAttr 0x60cb53088b10 <<invalid sloc>> Implicit R"ATTRDUMP(__ResolveAtRuntime)ATTRDUMP"
<<<NULL>>>

to

root [0] auto var = someexpr();
ROOT_prompt_0:1:12: error: Cannot deduce 'auto' from unknown expression
auto var = someexpr();
           ^

This Pull request:

Changes or fixes:

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Fixes #9261

CC: @guitargeek

@devajithvs devajithvs requested a review from hahnjo March 27, 2025 14:01
@devajithvs devajithvs self-assigned this Mar 27, 2025
@devajithvs devajithvs requested a review from dpiparo as a code owner March 27, 2025 14:01
@devajithvs devajithvs requested a review from vgvassilev March 27, 2025 14:01
Copy link

github-actions bot commented Mar 27, 2025

Test Results

    18 files      18 suites   4d 22h 33m 13s ⏱️
 2 737 tests  2 735 ✅ 0 💤 2 ❌
47 733 runs  47 731 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 9292714.

♻️ This comment has been updated with latest results.

@dpiparo
Copy link
Member

dpiparo commented Mar 27, 2025

CC: @silverweed

@devajithvs devajithvs force-pushed the dev.fix_9261 branch 3 times, most recently from 0d799ee to 45fb5c9 Compare March 28, 2025 05:37
Copy link
Member

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

Lgtm, if we add a test in clingtest.


// `auto` is not supported
auto x = unknownexpression()
// CHECK: {{input_line_[0-9]+:[0-9]+:[0-9]+}}: error: cannot deduce 'auto' from unknown expression
Copy link
Member

Choose a reason for hiding this comment

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

What about adding -verify and //expected-error?

Copy link
Contributor Author

@devajithvs devajithvs Mar 28, 2025

Choose a reason for hiding this comment

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

If we have -verify, we would have to change that for p.q too, I tried to do something like

p.q // expected-error {{use of undeclared identifier 'p'}}

// `auto` is not supported
auto x = unknownexpression(); // expected-error {{cannot deduce 'auto' from unknown expression}}

But I get this (the string is the exact same, but the line number is different)

error: 'expected-error' diagnostics expected but not seen: 
  File input_line_27 Line 2: use of undeclared identifier 'p'
error: 'expected-error' diagnostics seen but not expected: 
  File input_line_28 Line 2: use of undeclared identifier 'p'

Copy link
Member

Choose a reason for hiding this comment

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

You can check how the other diag checks are done in the tests. There is syntax for it although what you wrote should have worked

Copy link
Member

Choose a reason for hiding this comment

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

Is there a combination that guarantees coverage and works? We need pragmatism... It would be nice to have this fixed asap :)

Copy link
Member

Choose a reason for hiding this comment

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

As far as I understand, the file used FileCheck before. I agree it would be nice to use -verify, but I've seen other cases in the past where it was hard because of Cling line errors... Maybe this is one of the cases where you have to give the exact line number as // expected-error@input_line_28 {{cannot deduce 'auto' from unknown expression}} (and then sometimes suffer updating the line numbers when changing the test)

Replace AST dumps with proper error messages in dynamic lookup
when dealing with `auto`, which is unsupported for lookup

Changes this:
```
root [0] auto var = someexpr();
ROOT_prompt_0:1:1: error: Syntax error
auto var = someexpr();
^
FunctionDecl 0x60cb530887f0 <input_line_8:1:1, ROOT_prompt_0:3:1> input_line_8:1:6 __cling_Un1Qu30 'void (void *)'
|-ParmVarDecl 0x60cb53088738 <col:22, col:28> col:28 vpClingValue 'void *'
|-CompoundStmt 0x60cb53088c30 <col:42, ROOT_prompt_0:3:1>
| |-DeclStmt 0x60cb53088c10 <line:1:1, col:22>
| | `-VarDecl 0x60cb53088910 <col:1, col:21> col:6 var 'auto' cinit
| |   `-CallExpr 0x60cb53088bc0 <col:12, col:21> '<dependent type>'
| |     `-DeclRefExpr 0x60cb53088b80 <col:12> '<dependent type>' lvalue Var 0x60cb53088aa8 'someexpr' '<dependent type>'
| `-NullStmt 0x60cb53088c28 <line:2:1>
|-AnnotateAttr 0x60cb53088a00 <<invalid sloc>> Implicit R"ATTRDUMP(__ResolveAtRuntime)ATTRDUMP"
`-AnnotateAttr 0x60cb53088b10 <<invalid sloc>> Implicit R"ATTRDUMP(__ResolveAtRuntime)ATTRDUMP"
<<<NULL>>>
```
to

```
root [0] auto var = someexpr();
ROOT_prompt_0:1:12: error: Cannot deduce 'auto' from unknown expression
auto var = someexpr();
           ^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cling][DF] auto firstTwo = Take(v, 2) causes error
5 participants