[BoundsSafety] Delay processing of bounds attrs in templates #9929
Conversation
hnrklssn
added
the
clang:bounds-safety
| @@ -3485,6 +3485,9 @@ namespace clang { | |||
|
|
|||
| void DynamicCountPointerAssignmentAnalysis::run() { | |||
| AnalysisDeclContext AC(/* AnalysisDeclContextManager */ nullptr, dcl); | |||
| if (isa<TemplateDecl>(dcl)) // delay processing until template has been | |||
| // instantiated | |||
| return; | |||
There was a problem hiding this comment.
How does it work? Will DynamicCountPointerAssignmentAnalysis::run be called again when dcl is instantiated?
There was a problem hiding this comment.
Yes, a new function decl will be created for the instantiated version and all the normal analyses will be run over it
There was a problem hiding this comment.
Is this sufficient? We could have dependent expressions when the declaration itself is not templated:
- Non-templated method of a templated struct referencing the template argument
- Lambda inside a templated function referencing the template argument
- Method of a non-templated struct nested inside a templated struct referencing the outer struct's template argument
There was a problem hiding this comment.
Changed to an isTemplated call instead
|
CC @ziqingluo-90 since we were discussing templates recently. |
|
@swift-ci please test |
|
@swift-ci please test llvm |
| @@ -3485,6 +3485,9 @@ namespace clang { | |||
|
|
|||
| void DynamicCountPointerAssignmentAnalysis::run() { | |||
| AnalysisDeclContext AC(/* AnalysisDeclContextManager */ nullptr, dcl); | |||
| if (isa<TemplateDecl>(dcl)) // delay processing until template has been | |||
| // instantiated | |||
| return; | |||
There was a problem hiding this comment.
Is this sufficient? We could have dependent expressions when the declaration itself is not templated:
- Non-templated method of a templated struct referencing the template argument
- Lambda inside a templated function referencing the template argument
- Method of a non-templated struct nested inside a templated struct referencing the outer struct's template argument
clang/lib/Sema/SemaDeclAttr.cpp
Outdated
| static void handlePtrCountedByEndedByAttr(Sema &S, Decl *D, | ||
| const ParsedAttr &AL) { | ||
| unsigned Level; | ||
| if (!S.checkUInt32Argument(AL, AL.getArgAsExpr(1), Level)) | ||
| return; | ||
|
|
||
| if (D->getDescribedTemplate() || S.CurContext->isDependentContext()) { |
There was a problem hiding this comment.
Same question here, wondering if this is sufficient for non-templated entities nested in templates.
There was a problem hiding this comment.
Also replaced with a call to isTemplated
Dynamic bounds attributes do not handle value dependent arguments. To enable wider interop with C++ code bases we delay the processing of these attributes inside templated contexts. Instead we apply the new type while instantiating the function. One issue with this approach is that instantiation happens after parsing is finished, and the Scope information we use to prevent attributes from referring to arguments from outer scopes is only available during parsing. These diagnostics were emitted at the end of the type processing. In templated contexts we instead perform that analysis during parsing, and delay the rest of the processing. To avoid churn in unrelated tests the rest of the attributes keep their processing as is, until we've investigated what impact hoisting it would have on the user experience.
hnrklssn
force-pushed
the
bounds-safety-value-dependence
branch
from
427922f to
9858ef7
Compare
|
@swift-ci please test llvm |
|
@swift-ci please smoke test |
Dynamic bounds attributes do not handle value dependent arguments. To enable wider interop with C++ code bases we delay the processing of these attributes inside templated contexts. Instead we apply the new type while instantiating the function.
One issue with this approach is that instantiation happens after parsing is finished, and the Scope information we use to prevent attributes from referring to arguments from outer scopes is only available during parsing. These diagnostics were emitted at the end of the type processing. In templated contexts we instead perform that analysis during parsing, and delay the rest of the processing. To avoid churn in unrelated tests the rest of the attributes keep their processing as is, until we've investigated what impact hoisting it would have on the user experience.