Skip to content

[BUG] Cppfront reports incorrect errors when a multi-statement function has no enclosing braces #946

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

Closed
bluetarpmedia opened this issue Jan 17, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@bluetarpmedia
Copy link
Contributor

Describe the bug
Cppfront reports an incorrect error when parsing an ill-formed multi-statement function missing its enclosing braces.

To Reproduce
Run cppfront on this ill-formed code:

func: () =
    message := "Hello world\n";
    std::cout << message;

Godbolt

Cppfront reports: error: local variable message is not used; consider changing its name to '_' to make it explicitly anonymous, or removing it entirely if its side effects are not needed

Another:

func: () =
    std::cout << "Hello ";
    std::cout << " world\n";

Godbolt

Cppfront reports: error: pure-cpp2 switch disables Cpp1 syntax

But in these cases I'd expect a syntax error indicating the block is missing.

@bluetarpmedia bluetarpmedia added the bug Something isn't working label Jan 17, 2024
@JohelEGP
Copy link
Contributor

They both look correct to me.
The last line is Cpp1.
So in the first case, the local really is unused.
And in the second case, there is Cpp1 code.

@JohelEGP
Copy link
Contributor

Is there any risk to parsing one more statement to diagnose this?

@bluetarpmedia
Copy link
Contributor Author

They both look correct to me. The last line is Cpp1. So in the first case, the local really is unused. And in the second case, there is Cpp1 code.

Yeah, you're right. Here's an example only with Cpp2 code:

main: () -> int =
    s: std::string = "Fred";
    myfile := fopen("xyzzy", "w");
    _ = myfile.fprintf( "Hello %s with UFCS!", s.c_str() );
    _ = myfile.fclose();

Error: error: local variable s is not used...

@bluetarpmedia
Copy link
Contributor Author

For context, I was playing with the pure2-main-args.cpp2 regression test, which looks like this:

main: (args) =
    std::cout
        << "args.argc            is (args.argc)$\n"
        << "args.argv[0]         is (args.argv[0])$\n"
        ;

And I added a local variable at the top:

main: (args) =
    message:= "Hello world\n";
    std::cout << message;
    std::cout
        << "args.argc            is (args.argc)$\n"
        << "args.argv[0]         is (args.argv[0])$\n"
        ;

And then received the error about message being unused, which confused me until I realized the block was missing.

@hsutter
Copy link
Owner

hsutter commented Jan 18, 2024

Right, at the end of a top-level Cpp2 declaration, if there isn't the start of another Cpp2 declaration we assume it's Cpp1 code and switch back to Cpp1 code. To give a different error on that would require a more fundamental change to how we recognize Cpp1/Cpp2 code which is the earliest stage of processing.

However, if it's nested inside a Cpp2 region such as a namespace, you do get a better error already:

ns: namespace = {
    func: () =
        std::cout << "Hello";
        std::cout << "world";
}

This program already got this error, which I think was fairly decent:

demo.cpp2(4,9): error: a namespace scope must contain only declarations or 'using' statements, not other code

But with the above commit, it now gives this better message:

demo.cpp2(4,9): error: in this scope, a single-statement function body cannot be immediately followed by a statement - did you forget to put { } braces around a multi-statement function body?

Again, this only fires if it's not a global function.

@JohelEGP
Copy link
Contributor

JohelEGP commented Oct 10, 2024

Is there any risk to parsing one more statement to diagnose this?

Well, there was, for #907.

x: ()      = y;
z: @w type = { }

The tentative parsing breaks the name lookup for @w.
It asserts to find only namespaces in current_declarations (excluding the current declaration).
But it finds x, which isn't a namespace.

#907's name lookup certainly needs work.
But bugs might lurk ahead when combining non-local state and tentative parsing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants