I don't think this is fully what I am trying to fix with this patch. This patch addresses the inconsistency that the CXXConstructExpr contains the correct ParenRange for Stmts like:
X value(7); But an incorrect range if the constructor is called like a function, e.g.: X(7); As for printing, I don't know whether the SourceRange of the CXXConstructExpr should include the parentheses. However, for refactoring, it is definitely highly beneficial to know it (and in fact I think we'll have to go and add it to other nodes as well). So, as CXXConstructExpr already contains a member for that, we should definitely populate it as best we can. On Tue, Jul 10, 2012 at 11:58 PM, Richard Smith <[email protected]>wrote: > On Mon, Jul 9, 2012 at 6:24 AM, Daniel Jasper <[email protected]> wrote: > >> I have noticed that the CXXConstructExpr is not always created with the >> correct parenthesis range. For example: >> >> struct X { X(int x) {} }; >> void f() { >> X value(7); // CXXConstructExpr has correct range (5, 12) >> X(7); // CXXConstructExpr has incorrect range (3, 5), does not include >> ")" >> } >> >> Although I am not 100% sure this is a bug and not a feature, the attached >> patch fixes it. Please take a look. >> > > Here's the source ranges we currently give to the CXXConstructExpr: > > X value; > ^~~~~ > X value(7); > ^~~~~~~~ > X value{7}; -ast-print produces "X value7;"! > ^~~~~~~ > X value = 7; > ^~~~~~~~~ (for 'value' copy constructor call) > ^ (for X(7) temporary constructor call) > X value = {7}; -ast-print produces "X value = 7;"! > ^~ > X value = X(7); > ^~~~ (for 'value' copy constructor call) > ^~~ (for 'X(7)' temporary constructor call) > X value = X{7}; -ast-print produces "X value = X(7);"! > ^~~~ (for 'value' copy constructor call) > ^ end loc invalid! (for 'X{7}' CXXTemporaryObjectExpr) > > For non-CXXConstructExpr initializations, here are the source ranges for > the initializers: > > int value = 7; > ^ > int value(7); > ^ > int value{7}; > ^~~ > int value = int(7); > ^~~~~~ > int value = int{7}; // -ast-print prints as int({ 7 })"! > ^ end loc invalid! > ^~~ (for InitListExpr within the CXXFunctionalCastExpr) > > I don't like changing the source ranges here without a clear picture of > what is "right". To that end, my proposal is that the set of tokens in > the source range for a node should match the set of tokens printed out by > the AST printer for that node; that also has the advantage that we could > easily write tests which cover both the AST printer and source ranges, > without having to annotate where anything is supposed to start and end. > > For a CXXConstructExpr, that would mean that enclosing parens are not part > of the source range (but that enclosing braces are). That inconsistency > seems unfortunate, but it is much more consistent than our current results. > > (Note that we don't actually print the braces as part of a > CXXConstructExpr yet -- that's because the StmtPrinter can't tell that > braced initialization was used, since we fail to set the > IsListInitialization flag when creating a CXXConstructExpr -- see the FIXME > in BuildCXXConstructExpr in SemaDeclCXX for that.) >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
