Identify constant definitions that look like include guard definitions #26098
Conversation
lib/ClangImporter/ImportName.cpp
Outdated
| && macro->getNumTokens() == 1 | ||
| && macro->tokens()[0].isLiteral()) { | ||
| auto* tok = macro->tokens()[0].getLiteralData(); | ||
| if (tok && strlen(tok) == 1 && strncmp("1", tok, 1) == 0) { |
There was a problem hiding this comment.
Seems like this might be simpler if you wrap tok in a StringRef
That way you can just use if (tok == "1")
| @@ -176,3 +176,8 @@ func testNulls() { | |||
| let _: Int = DEPRECATED_ONE // expected-error {{use of unresolved identifier 'DEPRECATED_ONE'}} | |||
| let _: Int = OKAY_TYPED_ONE // expected-error {{cannot convert value of type 'okay_t' (aka 'UInt32') to specified type 'Int'}} | |||
| } | |||
|
|
|||
| func testHeaderGuard() { | |||
| _ = IS_HEADER_GUARD // expected-error {{use of unresolved identifier 'IS_HEADER_GUARD'}} | |||
There was a problem hiding this comment.
Typo: Seems like this should be IS_A_HEADER_GUARD.
lib/ClangImporter/ImportName.cpp
Outdated
| if (macro->isUsedForHeaderGuard() | ||
| && macro->getNumTokens() == 1 | ||
| && macro->tokens()[0].isLiteral()) { | ||
| auto* tok = macro->tokens()[0].getLiteralData(); |
There was a problem hiding this comment.
Nitpick: We try to only use auto when the type is meaningless or obvious from the right-hand expression; in this case explicitly saying const char * is probably more appropriate. Also, the star should be attached to the variable name to match C's parsing rules.
There was a problem hiding this comment.
I got rid of that auto* in the newer version. I'm still using auto though because I think the result type of the array access is clear.
lib/ClangImporter/ImportName.cpp
Outdated
| && macro->getNumTokens() == 1 | ||
| && macro->tokens()[0].isLiteral()) { | ||
| auto* tok = macro->tokens()[0].getLiteralData(); | ||
| if (tok && strlen(tok) == 1 && strncmp("1", tok, 1) == 0) { |
There was a problem hiding this comment.
LLVM has a very useful type, StringRef, that makes working with strings a lot easier. So this code can be written if (tok && StringRef(tok) == "1")
lib/ClangImporter/ImportName.cpp
Outdated
| // Ignore include guards. Try not to ignore definitions of useful constants, | ||
| // which may end up looking like include guards. | ||
| if (macro->isUsedForHeaderGuard() && macro->getNumTokens() == 1) { | ||
| auto tok = macro->tokens()[0]; |
There was a problem hiding this comment.
…extra level of indentation snuck in this time too.
|
@swift-ci please smoke test |
|
Once I have the fixes for CI, I'll open a new PR from my GitHub account for work @varungandhi-apple. Closing for now. |
The solution follows the outline in the issue description. If the literal "1" is present,
we treat the construct as an include guard, otherwise it is treated as defining a constant.
Resolves SR-9482.