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.

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

Casts broken in ROOT prompt #8304

Closed
1 task done
eguiraud opened this issue Jun 1, 2021 · 6 comments · Fixed by #8555
Closed
1 task done

Casts broken in ROOT prompt #8304

eguiraud opened this issue Jun 1, 2021 · 6 comments · Fixed by #8555

Comments

@eguiraud
Copy link
Member

eguiraud commented Jun 1, 2021

  • Checked for duplicates

Describe the bug

Weird/wrong results printed when converting different entities to bool:

root [0] int foo = 42
(int) 42
root [1] (bool)foo
(bool) true
root [2] bool(foo)
(bool) false
root [0] const char *foo = "asda"
(const char *) "asda"
root [1] !foo
(bool) false
root [2] bool(foo)
(bool) false
root [3] bool(foo[0])
(bool ([0]) @0x55c4ba66f180
@Axel-Naumann
Copy link
Member

Also note that the last example is missing a closing ) for the type...

@jalopezg-git
Copy link
Collaborator

jalopezg-git commented Jun 21, 2021

Hi @eguiraud and @Axel-Naumann,

Actually, this is an instance of vexing parse. Specifically, in the context where bool(foo) appears, it is parsed as a variable declaration with superfluous parenthesis, i.e. same as bool foo. Because definition shadowing is enabled, it replaces the old definition (of type int) and value-initializes it to false, which explains your first reported result.
Moreover, using !!bool(foo) disambiguates the input, which is now parsed as an expression and correctly prints true.

The behavior seen in the second report is again due to vexing parse, and bool(foo[0]) is parsed as the declaration bool foo[0];. Again, switching off definition shadowing gives:

ROOT_prompt_3:1:6: error: redefinition of 'foo' with a different type: 'bool ([0]' vs 'const char *'
bool(foo[0])
     ^
ROOT_prompt_1:1:13: note: previous definition is here
const char *foo = "foo";

In this case, the output (bool ([0]) @0x55c4ba66f180 is the address of the bool [0] object (whose size should be 1).

Also note that the last example is missing a closing ) for the type...

Also, as can be seen above, the diagnostic message provided by Clang has one extra (, which I think should not be there.

Now, what can we do about this? IMHO, while I think that is the correct parsing, we should be probably issuing a warning. Any opinions on this?

Cheers,
Javier.

jalopezg-git added a commit to jalopezg-git/root that referenced this issue Jun 28, 2021
Warn on redundant parentheses in declarators whose parsing might not match
the user intent, e.g.
```
root [0] int i = 1;
root [1] (bool)i
(bool) true
root [2] bool(i)
ROOT_prompt_1:1:5: warning: redundant parentheses surrounding declarator [-Wredundant-parens]
bool(i)
    ^~~
(bool) false
```

For more information see http://github.com/root-project/issues/8304.
Closes issue root-project#8304.
jalopezg-git added a commit to jalopezg-git/root that referenced this issue Jul 2, 2021
Warn on redundant parentheses in declarators whose parsing might not match
the user intent, e.g.
```
root [0] int i = 1;
root [1] (bool)i
(bool) true
root [2] bool(i)
ROOT_prompt_1:1:5: warning: redundant parentheses surrounding declarator [-Wredundant-parens]
bool(i)
    ^~~
(bool) false
```

For more information see http://github.com/root-project/issues/8304.
Closes issue root-project#8304.
jalopezg-git added a commit to jalopezg-git/root that referenced this issue Jul 5, 2021
Warn on redundant parentheses in declarators whose parsing might not match
the user intent, e.g.
```
root [0] int i = 1;
root [1] (bool)i
(bool) true
root [2] bool(i)
ROOT_prompt_1:1:5: warning: redundant parentheses surrounding declarator [-Wredundant-parens]
bool(i)
    ^~~
(bool) false
```

For more information see http://github.com/root-project/issues/8304.
Closes issue root-project#8304.
jalopezg-git added a commit that referenced this issue Jul 9, 2021
Warn on redundant parentheses in declarators whose parsing might not match
the user intent, e.g.
```
root [0] int i = 1;
root [1] (bool)i
(bool) true
root [2] bool(i)
ROOT_prompt_1:1:5: warning: redundant parentheses surrounding declarator [-Wredundant-parens]
bool(i)
    ^~~
(bool) false
```

For more information see http://github.com/root-project/issues/8304.
Closes issue #8304.
pzhristov pushed a commit to alisw/root that referenced this issue Aug 27, 2021
Warn on redundant parentheses in declarators whose parsing might not match
the user intent, e.g.
```
root [0] int i = 1;
root [1] (bool)i
(bool) true
root [2] bool(i)
ROOT_prompt_1:1:5: warning: redundant parentheses surrounding declarator [-Wredundant-parens]
bool(i)
    ^~~
(bool) false
```

For more information see http://github.com/root-project/issues/8304.
Closes issue root-project#8304.
@jalopezg-git jalopezg-git reopened this Jan 26, 2022
@Axel-Naumann Axel-Naumann changed the title Conversion to bool has incorrect behavior in ROOT prompt Casts broken in ROOT prompt Jan 26, 2022
@devajithvs
Copy link
Contributor

As @jalopezg-git mentioned, bool(foo) appears as a declaration (bool foo).

The following simple C++ code also fails to compile with a re-declaration error and I think that should be the expected behavior.

#include<iostream>
int main() {
        int i = 54;
        bool(i);
}

@guitargeek
Copy link
Contributor

Now, what can we do about this? IMHO, while I think that is the correct parsing, we should be probably issuing a warning. Any opinions on this?

My opinion is if the user employs C-style casts and not e.g. static_cast, then it's on them 🙂

@pcanal
Copy link
Member

pcanal commented Mar 26, 2024

The challenge here is that the code does silenty the 'wrong' thing. The compiler and the case where shadowing is disabled, correctly fail with a good error. While the current prompt will do the wrong thing completely. On the other hand the issue is likely not severe as this only affect the prompt (where one would possibly cast to get a 'better' printout) and would not affect scripts. (I.e. despite being silent, the user might still notice).

@guitargeek
Copy link
Contributor

On the other the issue is likely not severe as this only affect the prompt (where one would possibly cast to get a 'better' printout) and would not affect scripts. (I.e. despite being silent, the user might still notice).

Exactly, it's not severe for many reasons, and it's not even wrong behavior given clings re-declaration feature.

Indeed a warning in this case would be nice-to-have, but the issue claims "casts broken in ROOT prompt", which is not the case.

@guitargeek guitargeek closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
6 participants