This revision was automatically updated to reflect the committed changes.
Closed by commit rL310616: Place implictly declared functions at block scope
(authored by chill).
Changed prior to commit:
https://reviews.llvm.org/D33676?vs=109366=110587#toc
Repository:
rL LLVM
chill added a comment.
Thanks for the review, I'll wait a few days.
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill updated this revision to Diff 109366.
chill added a comment.
Updated with only whitespace changes after formating each changed line with
clang-format.
https://reviews.llvm.org/D33676
Files:
include/clang/Sema/Scope.h
lib/Parse/ParseCXXInlineMethods.cpp
lib/Parse/ParseDecl.cpp
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
This looks reasonable to me, once it's been formatted. Let's give @rsmith a few
days to comment before committing, though.
https://reviews.llvm.org/D33676
chill added inline comments.
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:521-522
// to be re-used for method bodies as well.
- ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope);
+ ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope|
+
aaron.ballman added inline comments.
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:521-522
// to be re-used for method bodies as well.
- ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope);
+ ParseScope FnScope(this, Scope::FnScope|Scope::DeclScope|
+
chill added a comment.
Ping.
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping.
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping.
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill updated this revision to Diff 106056.
chill added a comment.
Herald added a subscriber: eraman.
Set the compound statement flag on all compound statement scopes (previous
version used to set the flag on just enough scopes
as to be sufficient for the purpose of inserting C90 implicit
chill added inline comments.
Comment at: lib/Parse/ParseStmt.cpp:843-845
+ return ParseCompoundStatement(isStmtExpr,
+Scope::DeclScope | Scope::CompoundStmtScope);
}
rsmith wrote:
> This seems to miss quite a lot of places that
rsmith added inline comments.
Comment at: lib/Parse/ParseStmt.cpp:843-845
+ return ParseCompoundStatement(isStmtExpr,
+Scope::DeclScope | Scope::CompoundStmtScope);
}
This seems to miss quite a lot of places that introduce
chill added a comment.
Ping.
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chill added a comment.
Ping?
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rogfer01 added a comment.
Thanks Momchil, this looks sensible to me.
What do you think @aaron.ballman @rsmith ?
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
chill added inline comments.
Comment at: test/Sema/implicit-decl.c:12
int32_t compCount = 0;
- if (_CFCalendarDecomposeAbsoluteTimeV(compDesc, vector, compCount)) { //
expected-note {{previous implicit declaration is here}} \
- expected-error {{implicit
chill updated this revision to Diff 102802.
chill added a comment.
Update 2: added a testcase involving expression statements
https://reviews.llvm.org/D33676
Files:
include/clang/Sema/Scope.h
lib/Parse/ParseStmt.cpp
lib/Sema/SemaDecl.cpp
test/Sema/implicit-decl-c90.c
rogfer01 added a comment.
Given that we are already considering extensions, maybe you want to add a test
for compound statements in expressions.
void foo(void)
{
({ bar(); });
int (*p)() = bar; /* expected-error {{use of undeclared identifier 'bar'}}
*/
}
This already works with
chill updated this revision to Diff 101522.
chill added a comment.
Update 1 notes.
The scopes for compound statements are explicitly marked as such, so then
`Sema::ImplicitylDefineFunction` can ascend to the nearest enclosing scope. The
function scopes (ones with `Scope:FnScope`) do not
rogfer01 added a comment.
I think what happens is that the implicit declaration is done in the prototype
scope which is not visible later.
Doing the same as GCC in the handling of this extension is probably the best:
note that in C90 a function call should not happen inside an array expression
chill added a comment.
In https://reviews.llvm.org/D33676#767746, @rogfer01 wrote:
> I understand that the scope `S` in this case is a (function) prototype scope
> so it would not be the innermost block scope, would it? That said, GCC does
> not accept `p_ok` above so probably this behaviour
rogfer01 added a comment.
I wonder what should we do in C99 for cases like
void foo(void) {
{
extern void g(int(*a)[call_to_undeclared()]);
int (*p_ok)() = call_to_undeclared;
}
int (*p_err)() = call_to_undeclared;
}
I understand that the scope `S` in this case
chill added a subscriber: cfe-commits.
chill added a comment.
This issue https://bugs.llvm.org//show_bug.cgi?id=2266 does not appear to
trigger anymore, with this patch applied.
https://reviews.llvm.org/D33676
___
cfe-commits mailing list
23 matches
Mail list logo
