From 9f371d2e72229a0160768050b85016968f26a236 Mon Sep 17 00:00:00 2001 From: Federico Fissore Date: Thu, 8 Oct 2015 12:22:59 +0200 Subject: [PATCH 1/6] Comparing prototypes with original functions. If they don't match, prototype is skipped Signed-off-by: Federico Fissore --- src/arduino.cc/builder/ctags_parser.go | 19 +++++++++++++++++++ .../TestCTagsParserFunctionPointers.txt | 5 +++++ .../builder/test/ctags_parser_test.go | 18 ++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 src/arduino.cc/builder/test/ctags_output/TestCTagsParserFunctionPointers.txt diff --git a/src/arduino.cc/builder/ctags_parser.go b/src/arduino.cc/builder/ctags_parser.go index e0448dcb..aad557bf 100644 --- a/src/arduino.cc/builder/ctags_parser.go +++ b/src/arduino.cc/builder/ctags_parser.go @@ -77,6 +77,7 @@ func (s *CTagsParser) Run(context map[string]interface{}) error { tags = addPrototypes(tags) tags = removeDefinedProtypes(tags) tags = removeDuplicate(tags) + tags = skipTagsWhere(tags, prototypeAndCodeDontMatch) if len(tags) > 0 { line, err := strconv.Atoi(tags[0][FIELD_LINE]) @@ -172,6 +173,24 @@ func signatureContainsDefaultArg(tag map[string]string) bool { return strings.Contains(tag[FIELD_SIGNATURE], "=") } +func prototypeAndCodeDontMatch(tag map[string]string) bool { + if tag[FIELD_SKIP] == "true" { + return true + } + + code := removeSpacesAndTabs(tag[FIELD_CODE]) + prototype := removeSpacesAndTabs(tag[KIND_PROTOTYPE]) + prototype = prototype[0 : len(prototype)-1] + + return strings.Index(code, prototype) == -1 +} + +func removeSpacesAndTabs(s string) string { + s = strings.Replace(s, " ", "", -1) + s = strings.Replace(s, "\t", "", -1) + return s +} + func filterOutTagsWithField(tags []map[string]string, field string) []map[string]string { var newTags []map[string]string for _, tag := range tags { diff --git a/src/arduino.cc/builder/test/ctags_output/TestCTagsParserFunctionPointers.txt b/src/arduino.cc/builder/test/ctags_output/TestCTagsParserFunctionPointers.txt new file mode 100644 index 00000000..0aa1e06a --- /dev/null +++ b/src/arduino.cc/builder/test/ctags_output/TestCTagsParserFunctionPointers.txt @@ -0,0 +1,5 @@ +setup /tmp/test907446433/preproc/ctags_target.cpp /^void setup(){$/;" kind:function line:2 signature:() returntype:void +loop /tmp/test907446433/preproc/ctags_target.cpp /^void loop(){}$/;" kind:function line:5 signature:() returntype:void +func /tmp/test907446433/preproc/ctags_target.cpp /^void (*func())(){$/;" kind:function line:7 signature:() returntype:void +funcArr /tmp/test907446433/preproc/ctags_target.cpp /^int (&funcArr())[5]{$/;" kind:function line:11 signature:() returntype:int +funcCombo /tmp/test907446433/preproc/ctags_target.cpp /^void (*(&funcCombo(void (*(&in)[5])(int)))[5])(int){$/;" kind:function line:15 signature:(void (*(&in)[5])(int)) returntype:void diff --git a/src/arduino.cc/builder/test/ctags_parser_test.go b/src/arduino.cc/builder/test/ctags_parser_test.go index 441c0ba9..298e8a72 100644 --- a/src/arduino.cc/builder/test/ctags_parser_test.go +++ b/src/arduino.cc/builder/test/ctags_parser_test.go @@ -243,3 +243,21 @@ func TestCTagsParserNamespace(t *testing.T) { require.Equal(t, "void setup();", prototypes[0].Prototype) require.Equal(t, "void loop();", prototypes[1].Prototype) } + +func TestCTagsParserFunctionPointers(t *testing.T) { + context := make(map[string]interface{}) + + bytes, err := ioutil.ReadFile(filepath.Join("ctags_output", "TestCTagsParserFunctionPointers.txt")) + NoError(t, err) + + context[constants.CTX_CTAGS_OUTPUT] = string(bytes) + + ctagsParser := builder.CTagsParser{PrototypesField: constants.CTX_PROTOTYPES} + ctagsParser.Run(context) + + prototypes := context[constants.CTX_PROTOTYPES].([]*types.Prototype) + + require.Equal(t, 2, len(prototypes)) + require.Equal(t, "void setup();", prototypes[0].Prototype) + require.Equal(t, "void loop();", prototypes[1].Prototype) +} From 830f8f20796c014605c07099e4190047af468105 Mon Sep 17 00:00:00 2001 From: Federico Fissore Date: Thu, 8 Oct 2015 14:57:07 +0200 Subject: [PATCH 2/6] Extracted function for readability Signed-off-by: Federico Fissore --- src/arduino.cc/builder/ctags_parser.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/arduino.cc/builder/ctags_parser.go b/src/arduino.cc/builder/ctags_parser.go index aad557bf..be4e0f3e 100644 --- a/src/arduino.cc/builder/ctags_parser.go +++ b/src/arduino.cc/builder/ctags_parser.go @@ -180,11 +180,15 @@ func prototypeAndCodeDontMatch(tag map[string]string) bool { code := removeSpacesAndTabs(tag[FIELD_CODE]) prototype := removeSpacesAndTabs(tag[KIND_PROTOTYPE]) - prototype = prototype[0 : len(prototype)-1] + prototype = removeTralingSemicolon(prototype) return strings.Index(code, prototype) == -1 } +func removeTralingSemicolon(s string) string { + return s[0 : len(s)-1] +} + func removeSpacesAndTabs(s string) string { s = strings.Replace(s, " ", "", -1) s = strings.Replace(s, "\t", "", -1) From bb83289e95a35a68e4f5fa9fd830cb0f35e108c9 Mon Sep 17 00:00:00 2001 From: Federico Fissore Date: Tue, 27 Oct 2015 18:23:27 +0100 Subject: [PATCH 3/6] Testing that prototype of a function containing a typename is not generated Signed-off-by: Federico Fissore --- .../builder/test/prototypes_adder_test.go | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/arduino.cc/builder/test/prototypes_adder_test.go b/src/arduino.cc/builder/test/prototypes_adder_test.go index 996b5554..5119f663 100644 --- a/src/arduino.cc/builder/test/prototypes_adder_test.go +++ b/src/arduino.cc/builder/test/prototypes_adder_test.go @@ -658,3 +658,43 @@ func TestPrototypesAdderSketchWithUSBCON(t *testing.T) { require.Equal(t, "#include \n#line 1\n", context[constants.CTX_INCLUDE_SECTION].(string)) require.Equal(t, "void ciao();\nvoid setup();\nvoid loop();\n#line 3\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) } + +func TestPrototypesAdderSketchWithTypename(t *testing.T) { + DownloadCoresAndToolsAndLibraries(t) + + context := make(map[string]interface{}) + + buildPath := SetupBuildPath(t, context) + defer os.RemoveAll(buildPath) + + context[constants.CTX_HARDWARE_FOLDERS] = []string{filepath.Join("..", "hardware"), "hardware", "downloaded_hardware"} + context[constants.CTX_TOOLS_FOLDERS] = []string{"downloaded_tools"} + context[constants.CTX_FQBN] = "arduino:avr:leonardo" + context[constants.CTX_SKETCH_LOCATION] = filepath.Join("sketch_with_typename", "sketch.ino") + context[constants.CTX_BUILD_PROPERTIES_RUNTIME_IDE_VERSION] = "10600" + context[constants.CTX_LIBRARIES_FOLDERS] = []string{"libraries", "downloaded_libraries"} + context[constants.CTX_VERBOSE] = true + + commands := []types.Command{ + &builder.SetupHumanLoggerIfMissing{}, + + &builder.ContainerSetupHardwareToolsLibsSketchAndProps{}, + + &builder.ContainerMergeCopySketchFiles{}, + + &builder.ContainerFindIncludes{}, + + &builder.PrintUsedLibrariesIfVerbose{}, + &builder.WarnAboutArchIncompatibleLibraries{}, + + &builder.ContainerAddPrototypes{}, + } + + for _, command := range commands { + err := command.Run(context) + NoError(t, err) + } + + require.Equal(t, "#include \n#line 1\n", context[constants.CTX_INCLUDE_SECTION].(string)) + require.Equal(t, "void setup();\nvoid loop();\n#line 6\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) +} From 8bb7912cb9fc36358e4ba49b72447e9a27423618 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 30 Oct 2015 09:52:43 +0100 Subject: [PATCH 4/6] Be more conservative about what prototypes to add Previously, a prototype was added if the generated prototype was contained in the original source code line, to catch cases where ctags did not provide enough information to generate a completely correct prototype. However, this substring matching was still too liberal: if something unsupported was present at the start of the function definition (such as `static` in `static void func()`) the substring matching would still think the prototype was correct, but the compiler would complain. This commit changes this to use prefix matching: Only if the original source line starts with the generated prefix, a prototype is added. Ideally, the full definition is matched, but that requires knowing where it ends, which can be at a curly open, but also a comment start, etc., so better to just leave it at this. This means that inline functions and functions with attributes specified at the start will no longer get a prototype, so the TestPrototypesAdderSketchWithInlineFunction is modified accordingly. Signed-off-by: Matthijs Kooijman --- src/arduino.cc/builder/ctags_parser.go | 2 +- src/arduino.cc/builder/test/prototypes_adder_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/arduino.cc/builder/ctags_parser.go b/src/arduino.cc/builder/ctags_parser.go index be4e0f3e..d0773196 100644 --- a/src/arduino.cc/builder/ctags_parser.go +++ b/src/arduino.cc/builder/ctags_parser.go @@ -182,7 +182,7 @@ func prototypeAndCodeDontMatch(tag map[string]string) bool { prototype := removeSpacesAndTabs(tag[KIND_PROTOTYPE]) prototype = removeTralingSemicolon(prototype) - return strings.Index(code, prototype) == -1 + return strings.Index(code, prototype) != 0 } func removeTralingSemicolon(s string) string { diff --git a/src/arduino.cc/builder/test/prototypes_adder_test.go b/src/arduino.cc/builder/test/prototypes_adder_test.go index 5119f663..7dd886ca 100644 --- a/src/arduino.cc/builder/test/prototypes_adder_test.go +++ b/src/arduino.cc/builder/test/prototypes_adder_test.go @@ -574,7 +574,7 @@ func TestPrototypesAdderSketchWithInlineFunction(t *testing.T) { } require.Equal(t, "#include \n#line 1\n", context[constants.CTX_INCLUDE_SECTION].(string)) - require.Equal(t, "void setup();\nvoid loop();\nshort unsigned int testInt();\nint8_t testInline();\nuint8_t testAttribute();\n#line 1\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) + require.Equal(t, "void setup();\nvoid loop();\nshort unsigned int testInt();\n#line 1\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) } func TestPrototypesAdderSketchWithFunctionSignatureInsideIFDEF(t *testing.T) { From c53189256b8b818cd639007117ad7466259062fe Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 30 Oct 2015 09:59:43 +0100 Subject: [PATCH 5/6] Merge some prototype generation testcases All of these contain complex functions that should not get any prototypes, so merging them into a single testcase seems useful. Signed-off-by: Matthijs Kooijman --- .../builder/test/prototypes_adder_test.go | 47 ++----------------- .../sketch.ino | 6 ++- .../test/sketch_with_default_args/sketch.ino | 8 ---- 3 files changed, 8 insertions(+), 53 deletions(-) rename src/arduino.cc/builder/test/{sketch_with_inline_function => sketch_with_complex_prototypes}/sketch.ino (66%) delete mode 100644 src/arduino.cc/builder/test/sketch_with_default_args/sketch.ino diff --git a/src/arduino.cc/builder/test/prototypes_adder_test.go b/src/arduino.cc/builder/test/prototypes_adder_test.go index 7dd886ca..34555e4c 100644 --- a/src/arduino.cc/builder/test/prototypes_adder_test.go +++ b/src/arduino.cc/builder/test/prototypes_adder_test.go @@ -495,7 +495,7 @@ func TestPrototypesAdderSketchNoFunctions(t *testing.T) { require.Nil(t, context[constants.CTX_PROTOTYPE_SECTION]) } -func TestPrototypesAdderSketchWithDefaultArgs(t *testing.T) { +func TestPrototypesAdderSketchComplexFunctions(t *testing.T) { DownloadCoresAndToolsAndLibraries(t) context := make(map[string]interface{}) @@ -506,7 +506,7 @@ func TestPrototypesAdderSketchWithDefaultArgs(t *testing.T) { context[constants.CTX_HARDWARE_FOLDERS] = []string{filepath.Join("..", "hardware"), "hardware", "downloaded_hardware"} context[constants.CTX_TOOLS_FOLDERS] = []string{"downloaded_tools"} context[constants.CTX_FQBN] = "arduino:avr:leonardo" - context[constants.CTX_SKETCH_LOCATION] = filepath.Join("sketch_with_default_args", "sketch.ino") + context[constants.CTX_SKETCH_LOCATION] = filepath.Join("sketch_with_complex_prototypes", "sketch.ino") context[constants.CTX_BUILD_PROPERTIES_RUNTIME_IDE_VERSION] = "10600" context[constants.CTX_BUILT_IN_LIBRARIES_FOLDERS] = []string{"downloaded_libraries"} context[constants.CTX_OTHER_LIBRARIES_FOLDERS] = []string{"libraries"} @@ -533,48 +533,7 @@ func TestPrototypesAdderSketchWithDefaultArgs(t *testing.T) { } require.Equal(t, "#include \n#line 1\n", context[constants.CTX_INCLUDE_SECTION].(string)) - require.Equal(t, "void setup();\nvoid loop();\n#line 1\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) -} - -func TestPrototypesAdderSketchWithInlineFunction(t *testing.T) { - DownloadCoresAndToolsAndLibraries(t) - - context := make(map[string]interface{}) - - buildPath := SetupBuildPath(t, context) - defer os.RemoveAll(buildPath) - - context[constants.CTX_HARDWARE_FOLDERS] = []string{filepath.Join("..", "hardware"), "hardware", "downloaded_hardware"} - context[constants.CTX_TOOLS_FOLDERS] = []string{"downloaded_tools"} - context[constants.CTX_FQBN] = "arduino:avr:leonardo" - context[constants.CTX_SKETCH_LOCATION] = filepath.Join("sketch_with_inline_function", "sketch.ino") - context[constants.CTX_BUILD_PROPERTIES_RUNTIME_IDE_VERSION] = "10600" - context[constants.CTX_BUILT_IN_LIBRARIES_FOLDERS] = []string{"downloaded_libraries"} - context[constants.CTX_OTHER_LIBRARIES_FOLDERS] = []string{"libraries"} - context[constants.CTX_VERBOSE] = false - - commands := []types.Command{ - &builder.SetupHumanLoggerIfMissing{}, - - &builder.ContainerSetupHardwareToolsLibsSketchAndProps{}, - - &builder.ContainerMergeCopySketchFiles{}, - - &builder.ContainerFindIncludes{}, - - &builder.PrintUsedLibrariesIfVerbose{}, - &builder.WarnAboutArchIncompatibleLibraries{}, - - &builder.ContainerAddPrototypes{}, - } - - for _, command := range commands { - err := command.Run(context) - NoError(t, err) - } - - require.Equal(t, "#include \n#line 1\n", context[constants.CTX_INCLUDE_SECTION].(string)) - require.Equal(t, "void setup();\nvoid loop();\nshort unsigned int testInt();\n#line 1\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) + require.Equal(t, "void setup();\nvoid loop();\nshort unsigned int testSimple();\n#line 1\n", context[constants.CTX_PROTOTYPE_SECTION].(string)) } func TestPrototypesAdderSketchWithFunctionSignatureInsideIFDEF(t *testing.T) { diff --git a/src/arduino.cc/builder/test/sketch_with_inline_function/sketch.ino b/src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino similarity index 66% rename from src/arduino.cc/builder/test/sketch_with_inline_function/sketch.ino rename to src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino index f0a25e69..d6180e80 100644 --- a/src/arduino.cc/builder/test/sketch_with_inline_function/sketch.ino +++ b/src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino @@ -1,7 +1,7 @@ void setup() {} void loop() {} -short unsigned int testInt(){ +short unsigned int testSimple(){ } @@ -12,3 +12,7 @@ static inline int8_t testInline(){ __attribute__((always_inline)) uint8_t testAttribute() { } + +void testDefault(int x = 1) { + +} diff --git a/src/arduino.cc/builder/test/sketch_with_default_args/sketch.ino b/src/arduino.cc/builder/test/sketch_with_default_args/sketch.ino deleted file mode 100644 index 6a5ee7cc..00000000 --- a/src/arduino.cc/builder/test/sketch_with_default_args/sketch.ino +++ /dev/null @@ -1,8 +0,0 @@ -void test(int x = 1) { -} - -void setup() { -} - -void loop() { -} \ No newline at end of file From c128bfacf413530199606eab52b70b5b892fe19c Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 30 Oct 2015 10:01:49 +0100 Subject: [PATCH 6/6] Add prototype adding test for static functions Static functions should not get a prototype added, since ctags does not allow generating the correct prototype. Signed-off-by: Matthijs Kooijman --- .../builder/test/sketch_with_complex_prototypes/sketch.ino | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino b/src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino index d6180e80..a55579ac 100644 --- a/src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino +++ b/src/arduino.cc/builder/test/sketch_with_complex_prototypes/sketch.ino @@ -16,3 +16,7 @@ __attribute__((always_inline)) uint8_t testAttribute() { void testDefault(int x = 1) { } + +static void testStatic() { + +}