A bunch of more or less related issues

Hi @clang,

while working on AST processing I stumbled over the one or another little quirk. I like to point them out now before the list becomes too long:

1. Why StmtPrinter::Indent always indent with two spaces hence effectively doubling the indent? It cost me a while to find that out as it is IMHO unexprected.
2. What is the full expression according to ISO in "if (int a = expr)"? IMHO it is expr, but I'm not really sure since I saw CXXConditionDeclExpr.
3. When I try to rewrite a freestanding expression I have trouble with the semicolon. Example:
{ expr; }
The semicolon is obv. not included in the source range of expr. But the parent of expr is already the CompoundStmt meaning that there seems to be no place to hold the information about the semicolon at all. Actually an encapsulating helper statement would be needed or there is a more clever solution. But maybe I just have to take this as a known limitation.
4. Why takes the FloatingLiteral ctor the "isexact" bool variable as a pointer?
5. Is it possible to change the PrinterHelper::handledStmt function so that it takes the current indentation as a third argument?

That's for now. Maybe someone can shed some light on some of these points.

Best
Olaf Krzikalla

Hi @clang,

while working on AST processing I stumbled over the one or another
little quirk. I like to point them out now before the list becomes too long:

1. Why StmtPrinter::Indent always indent with two spaces hence
effectively doubling the indent? It cost me a while to find that out as
it is IMHO unexprected.

Because "two character spacing" is the "one true way" :slight_smile:

2. What is the full expression according to ISO in "if (int a = expr)"?
IMHO it is expr, but I'm not really sure since I saw CXXConditionDeclExpr.

This is a C++'ism, I'm not really sure.

3. When I try to rewrite a freestanding expression I have trouble with
the semicolon. Example:
{ expr; }
The semicolon is obv. not included in the source range of expr. But the
parent of expr is already the CompoundStmt meaning that there seems to
be no place to hold the information about the semicolon at all. Actually
an encapsulating helper statement would be needed or there is a more
clever solution. But maybe I just have to take this as a known limitation.

Yes, we don't model the semi right now. In theory we could handle it with a new StmtExpr class, but we don't have that, and probably don't want it for space reasons.

4. Why takes the FloatingLiteral ctor the "isexact" bool variable as a
pointer?

Good question, no idea, I'll fix it.

5. Is it possible to change the PrinterHelper::handledStmt function so
that it takes the current indentation as a third argument?

Someone else can probably answer this better than me,

-Chris

Using just the expr is not correct, consider this case:

class C {
public:
  C(bool b);
  operator bool();
};

void f() {
  if (C c = true) {
    
  }
}

the condition used for "if" is not 'true' but the return value of calling C::operator bool().

-Argiris

Hi @clang,

while working on AST processing I stumbled over the one or another
little quirk. I like to point them out now before the list becomes too long:

1. Why StmtPrinter::Indent always indent with two spaces hence
effectively doubling the indent? It cost me a while to find that out as
it is IMHO unexprected.

From a brief investigation, I think that's simply a bug; patch welcome.

2. What is the full expression according to ISO in "if (int a = expr)"?
IMHO it is expr, but I'm not really sure since I saw CXXConditionDeclExpr.

ISO isn't really relevant here, I think. The way the AST is
structured, the if statement contains an CXXConditionDeclExpr, which
references the decl; "expr" is simply the intialializer for "a".

3. When I try to rewrite a freestanding expression I have trouble with
the semicolon. Example:
{ expr; }
The semicolon is obv. not included in the source range of expr. But the
parent of expr is already the CompoundStmt meaning that there seems to
be no place to hold the information about the semicolon at all. Actually
an encapsulating helper statement would be needed or there is a more
clever solution. But maybe I just have to take this as a known limitation.

There has been some discussion of adding helpers for cases like this,
but nobody has stepped up to implement them; see also
http://llvm.org/bugs/show_bug.cgi?id=3765 .

4. Why takes the FloatingLiteral ctor the "isexact" bool variable as a
pointer?

No good reason, as far as I can tell; patch welcome.

5. Is it possible to change the PrinterHelper::handledStmt function so
that it takes the current indentation as a third argument?

That sounds fine; patch welcome.

-Eli

Hi @clang,

take this post as an answer to all responses.

Eli Friedman schrieb:

  

Hi @clang,

while working on AST processing I stumbled over the one or another
little quirk. I like to point them out now before the list becomes too long:

1. Why StmtPrinter::Indent always indent with two spaces hence
effectively doubling the indent? It cost me a while to find that out as
it is IMHO unexprected.
    
From a brief investigation, I think that's simply a bug; patch welcome.
  

OK. I hope it's not a big problem, since existing code works a little
bit different after the change, i.e. it doesn't conform to the 'one true
way' by default anymore ;).

2. What is the full expression according to ISO in "if (int a = expr)"?
IMHO it is expr, but I'm not really sure since I saw CXXConditionDeclExpr.
    
ISO isn't really relevant here, I think. The way the AST is
structured, the if statement contains an CXXConditionDeclExpr, which
references the decl; "expr" is simply the intialializer for "a".
  

This one I should elaborate: what I have here is a new function
ParentMap::getFullExpression. At the moment this function simply
traverses the parent statement tree until a non-expr statement is found.
Of course I want to have the function doing its thing right, that is
yield the full expression according to ISO. Thatswhy I'm asking: IMHO I
should handle CXXConditionDeclExpr like a non-expr statement, but I'm
only 99% sure.

For the rest I'm going to post patches in the near future (probably
despite the seimcolon issue).

Best
Olaf Krzikalla