clang-repl doesn't execute top-level code located inside a namespace

[p4vook]

Consider the following piece of code:

#include <stdlib.h>
namespace ns { exit(1); }

Currently clang-repl does not execute the code inside ns.
However, executing top-level code inside namespaces not only seems the logical thing to do, but it could also prove to be useful when implementing proper notebook integration: consider this example:

#include <iostream>
int n = 1; std::cout << n; // user executes notebook cell the first time
// user edits the cell and wants to re-execute it.
int n = 2; std::cout << n; // error: redefinition of 'n'

This code is, indeed, incorrect: C++ does not allow variable or function redefinition.
However, it could potentially be rewritten into working code using C++17 syntax for nested namespaces:

#include <iostream>
namespace n1 { int n = 1; std::cout << n; } // currently nothing
namespace n1::n2 { int n = 2; std::cout << n; } // still nothing, but this doesn't trigger redefinition, and n1::n2::n = 2

This does not require any messing with compiler internals, so should behave in a way that's more predictable.

[p4vook]

see compiler-research/xeus-clang-repl#72 for more detail on the motivation

[p4vook]

More detail: this is probably a bug, because ParseTopLevelStmtDecl() is getting called for statement inside namespace, but it's getting lost somewhere on the way later.
When it's checked for in HandleTopLevelDecl(), there aren't any TopLevelStmtDecl inside the declaration group.

[vgvassilev]

Hi @p4vook, I am a little worried in allowing to execute statements on namespace level. However, it generally is consistent with the way C++ models static initialization. The example below prints i1, i2.

extern "C" int printf(const char*,...);

int i1 = printf("i1\n");
namespace N {int i2 = printf("i2\n");}

int main() {
}

I'd prefer to keep this as a patch and if this feature enables really variable redefinition in Jupyter cells with clang-repl we can pursue it. What do you think?

cc: @AaronBallman

[p4vook]

Oops, I'm sorry for missing your question earlier.

Yes, I'm absolutely fine with it being in a separate patch, as this behavior could be controversial. Although it seems that the CleanUpPTU() bug, where clang does what it shouldn't do, is related to inserting some statements into wrong contexts (and simply checking for nullptr is quite a bad fix for that). I spent a few hours trying to figure out the correct context for insertion, but my changes in the PR break the for loop for some obscure reason.

I'd be glad if someone with a deeper knowledge of the Clang AST could provide some insights.